Closed
Bug 1243704
Opened 8 years ago
Closed 8 years ago
Serialise errors sent over IPC
Categories
(Remote Protocol :: Marionette, defect)
Remote Protocol
Marionette
Tracking
(firefox47 fixed)
RESOLVED
FIXED
mozilla47
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: ato, Assigned: ato)
References
Details
(Keywords: pi-marionette-server)
Attachments
(1 file)
Using the IPC construct `message.objects` to send Error prototypes over IPC from content to chrome in https://dxr.mozilla.org/mozilla-central/rev/d4213241bb796fdfa7a5ad4f1989e97b44474364/testing/marionette/proxy.js#134 blocks e10s-tests. We should serialise the errors on the content side and pass them to chrome before deserialising them there. This work is made possible by bug 1153828.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → ato
Blocks: 1238095
Status: NEW → ASSIGNED
Depends on: 1153828
Keywords: ateam-marionette-server
Assignee | ||
Comment 1•8 years ago
|
||
This fixes an instance of passing an Error prototype over the message manager as a CPOW. We solve this by marshaling the error, which is now done automatically by the new AsyncMessageChannel. It allows us to create an (almost) transparent promise-based interface between chrome- and content contexts. The patch also makes AsyncMessageChannel reusable on both sides of the message listener, but it's currently not used at its maximum potential because of the way the listener is architected. Review commit: https://reviewboard.mozilla.org/r/32801/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/32801/
Attachment #8713653 -
Flags: review?(dburns)
Assignee | ||
Comment 2•8 years ago
|
||
Comment on attachment 8713653 [details] MozReview Request: Bug 1243704 - Serialise errors sent over IPC; r?automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/32801/diff/1-2/
Comment 3•8 years ago
|
||
Comment on attachment 8713653 [details] MozReview Request: Bug 1243704 - Serialise errors sent over IPC; r?automatedtester https://reviewboard.mozilla.org/r/32801/#review29687 ::: testing/marionette/error.js:105 (Diff revision 2) > + throw new TypeError("Unserialisable error type: " + err); nit: We should probably use string templating to keep more inline with the way we are going throughout the code ::: testing/marionette/error.js:127 (Diff revision 2) > + throw new TypeError("Undeserialisable error type: " + json.error); nit: We should probably use string templating to keep more inline with the way we are going throughout the code ::: testing/marionette/proxy.js:123 (Diff revision 2) > + "Unknown async response type: " + msg.json.type); nit: We should probably use string templating to keep more inline with the way we are going throughout the code ::: testing/marionette/proxy.js:200 (Diff revision 2) > + return "Marionette:asyncReply:" + uuid; nit: We should probably use string templating to keep more inline with the way we are going throughout the code
Attachment #8713653 -
Flags: review?(dburns) → review+
Assignee | ||
Comment 4•8 years ago
|
||
https://reviewboard.mozilla.org/r/32801/#review29687 > nit: We should probably use string templating to keep more inline with the way we are going throughout the code Unless the variable is actually in the middle of a string, I’m dubious of the value of using template strings as they actually are harder to read and type. But I’m fine with changing it. > nit: We should probably use string templating to keep more inline with the way we are going throughout the code I’m dropping this because template strings comes with a performance penalty, and we shouldn’t use them where we need performant code. This particular statement gets executed twice for every call on the AsyncMessageChannel.
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 8713653 [details] MozReview Request: Bug 1243704 - Serialise errors sent over IPC; r?automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/32801/diff/2-3/
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c499016196da
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Updated•1 year ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•