Alexandre Poirot [:ochameau] Assignee
	Description • 7 years ago

We plan to move devtools to an add-on while moving the codebase to github, out of mozilla-central.
So, ideally, marionette shouldn't depend on devtools modules. It depends on transport.js from here:
http://searchfox.org/mozilla-central/source/testing/marionette/server.js#23
  loader.loadSubScript("resource://devtools/shared/transport/transport.js");

I think we should just put a copy of it in marionette. Marionette doesn't really depends Devtools. It just happen to reuse its protocol. But I don't think it cares about whatever update we could do to the protocol. It may even be easier for marionette to keep a fork and do not have to update whenever devtools decide to change something to the transport layer.

	Alexandre Poirot [:ochameau] Assignee
	Comment 1 • 7 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=546f0e6f038a5d9b24113c430812170325198c76

	Andreas Tolfsen ❲:ato❳
	Comment 2 • 7 years ago

(In reply to Alexandre Poirot [:ochameau] from comment #0)
> I think we should just put a copy of it in marionette. Marionette doesn't
> really depends Devtools. It just happen to reuse its protocol. But I don't
> think it cares about whatever update we could do to the protocol. It may
> even be easier for marionette to keep a fork and do not have to update
> whenever devtools decide to change something to the transport layer.

It’s worse than that: Marionette implements a custom message format described in https://developer.mozilla.org/en-US/docs/Mozilla/QA/Marionette/Protocol on top of transport.js.

I support the idea of copying transport.js over to Marionette if you intend on making devtools an add-on.  Feel free to r? me for a review.

	Alexandre Poirot [:ochameau] Assignee
	Comment 3 • 7 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a7dcb05617b628da43f35ba0693b2aa79bdd970

	Alexandre Poirot [:ochameau] Assignee
	Comment 4 • 7 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=82ffc28c12ff1ce921dd732f31abf508d0286d27

	Alexandre Poirot [:ochameau] Assignee
	Comment 6 • 7 years ago

This patch bundles today's version of the protocol. I had another version of this patch based on an older version of transport.js, before we introduced "bulk data messages". This old version was still a single file. I think it would work if we downgrade PROTOCOL_VERSION to 1 or 2... But I'm not sure that's something we want ?

So, this patch, comes with the two mandatory dependencies of transport.js: stream-utils and packets.
I got rid of any other useless dependencies, like some for special dump() calls.
I also removed some conditional dump completely.

	Andreas Tolfsen ❲:ato❳
	Comment 7 • 7 years ago

(In reply to Alexandre Poirot [:ochameau] from comment #6)
> This patch bundles today's version of the protocol. I had another version of
> this patch based on an older version of transport.js, before we introduced
> "bulk data messages". This old version was still a single file. I think it
> would work if we downgrade PROTOCOL_VERSION to 1 or 2... But I'm not sure
> that's something we want ?

Bundling/forking the HEAD of m-c makes sense.

> So, this patch, comes with the two mandatory dependencies of transport.js:
> stream-utils and packets. I got rid of any other useless dependencies, like
> some for special dump() calls. I also removed some conditional dump
> completely.

Thanks for doing this!

	Andreas Tolfsen ❲:ato❳
	Comment 8 • 7 years ago
mozreview-review

Comment on attachment 8861159 [details]
Bug 1356237 - Ship a copy of transport module in marionette.

https://reviewboard.mozilla.org/r/133112/#review135938

r+ if you expand the commit message per comment.

::: commit-message-73752:1
(Diff revision 1)
> +Bug 1356237 - Ship a copy of transport module in marionette. r=ato

Please expand on why we are doing this, e.g. that devtools is getting moved into an add-on and what consequences this has.

	Pulsebot
	Comment 10 • 7 years ago

Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/16a917f9afbc
Ship a copy of transport module in marionette. r=ato

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 11 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/16a917f9afbc