If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Proxy callback functions from the child process in UITour head.js

RESOLVED FIXED in Firefox 48

Status

()

Firefox
Tours
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Gijs, Assigned: Gijs)

Tracking

(Blocks: 1 bug)

44 Branch
Firefox 48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
We proxy the function calls, but any callback parameters, such as to getConfiguration, don't get dealt with, and the result is that things Don't Work in e10s.

This is fixable. :-)
(Assignee)

Comment 1

2 years ago
Created attachment 8730666 [details]
MozReview Request: Bug 1256625 - enable function parameters to gContentAPI wrappers to fix UITour tests in e10s mode, r?MattN

Review commit: https://reviewboard.mozilla.org/r/40077/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/40077/
Attachment #8730666 - Flags: review?(MattN+bmo)
(Assignee)

Comment 2

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ee7bf29992f
(Assignee)

Updated

2 years ago
Blocks: 1256746
Comment on attachment 8730666 [details]
MozReview Request: Bug 1256625 - enable function parameters to gContentAPI wrappers to fix UITour tests in e10s mode, r?MattN

https://reviewboard.mozilla.org/r/40077/#review37687

This is a lot of non-trivial code to implement this feature so before this patch I was leaning towards switching the tests which used callbacks to their equiavalent promise-based ContentTask helper instead. I also didn't really see the Proxy hack as a long-term solution (e.g. for new tests) but was a quick way to avoid rewriting existing tests. Since you already did the work I guess this is fine but if there are easy cases in the future where you can use an existing promise helper with ContentTask then please prefer that.
Attachment #8730666 - Flags: review?(MattN+bmo) → review+

Comment 4

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a6dca9d0ecdd

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a6dca9d0ecdd
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
You need to log in before you can comment on or make changes to this bug.