Closed Bug 1243704 Opened 8 years ago Closed 8 years ago

Serialise errors sent over IPC

Categories

(Remote Protocol :: Marionette, defect)

defect
Not set
normal

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: nobody → ato
Blocks: 1238095
Status: NEW → ASSIGNED
Depends on: 1153828
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)
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 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+
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.
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/
https://hg.mozilla.org/mozilla-central/rev/c499016196da
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: