Closed
Bug 1249332
Opened 8 years ago
Closed 8 years ago
[e10s] Make browser/modules/test/browser_ContentSearch.js work under e10s
Categories
(Firefox :: Search, defect)
Firefox
Search
Tracking
()
RESOLVED
FIXED
Firefox 47
People
(Reporter: adw, Assigned: adw)
References
(Blocks 2 open bugs)
Details
Attachments
(1 file, 1 obsolete file)
6.83 KB,
patch
|
mak
:
review+
|
Details | Diff | Splinter Review |
Make browser/modules/test/browser_ContentSearch.js work under e10s.
Updated•8 years ago
|
Blocks: e10s-tests
tracking-e10s:
--- → +
Assignee | ||
Comment 1•8 years ago
|
||
Marco, would you mind reviewing? The only e10s problem was the webprogresslistener used by the test to stop search pages from loading. This patch moves that into the content script and then has the content script tell the main test about the load, so that the test can continue.
Attachment #8721554 -
Flags: review?(mak77)
Assignee | ||
Comment 2•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=db3c5cf0fdcd
Updated•8 years ago
|
Attachment #8721554 -
Flags: review?(mak77) → review+
Assignee | ||
Comment 3•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/996485ab7306
Assignee | ||
Comment 4•8 years ago
|
||
Backed out for making bug 1188692 worse: https://hg.mozilla.org/integration/fx-team/rev/fd90f5266bd5
Assignee | ||
Comment 5•8 years ago
|
||
This is strange, the try results were fine (-u mochitest-bc,mochitest-e10s-bc). The failures on fx-team were actually due to a JS error in waitForLoadAndStopIt, preventing it from stopping the load:
16:16:05 INFO - JavaScript error: chrome://mochitests/content/browser/browser/modules/test/contentSearch.js, line 60: TypeError: docShell.addProgressListener is not a function
16:16:06 INFO - FATAL ERROR: Non-local network connections are disabled and a connection attempt to www.google.com (74.125.239.114) was made.
And the test passed for me locally in both e10s and non-e10s. But after refreshing my tree just now, non-e10s fails locally with the error above.
This new patch does a QI dance, like tab-content.js does for the same thing, and it passes locally. We'll see what try says this time.
Diff between this patch and the previous:
> diff --git a/browser/modules/test/contentSearch.js b/browser/modules/test/contentSearch.js
> --- a/browser/modules/test/contentSearch.js
> +++ b/browser/modules/test/contentSearch.js
> @@ -33,30 +33,33 @@ addMessageListener(TEST_MSG, msg => {
> url: url,
> });
> });
> }
> });
>
> function waitForLoadAndStopIt(expectedURL, callback) {
> let Ci = Components.interfaces;
> + let webProgress =
> + docShell.QueryInterface(Ci.nsIInterfaceRequestor).
> + getInterface(Ci.nsIWebProgress);
> let listener = {
> onStateChange: function (webProg, req, flags, status) {
> if (req instanceof Ci.nsIChannel) {
> let url = req.originalURI.spec;
> dump("waitForLoadAndStopIt: onStateChange " + url + "\n");
> let docStart = Ci.nsIWebProgressListener.STATE_IS_DOCUMENT |
> Ci.nsIWebProgressListener.STATE_START;
> if ((flags & docStart) && webProg.isTopLevel && url == expectedURL) {
> - docShell.removeProgressListener(listener);
> + webProgress.removeProgressListener(listener);
> req.cancel(Components.results.NS_ERROR_FAILURE);
> callback(url);
> }
> }
> },
> QueryInterface: XPCOMUtils.generateQI([
> Ci.nsIWebProgressListener,
> Ci.nsISupportsWeakReference,
> ]),
> };
> - docShell.addProgressListener(listener, Ci.nsIWebProgress.NOTIFY_ALL);
> + webProgress.addProgressListener(listener, Ci.nsIWebProgress.NOTIFY_ALL);
> dump("waitForLoadAndStopIt: Waiting for URL to load: " + expectedURL + "\n");
> }
Attachment #8721554 -
Attachment is obsolete: true
Attachment #8722267 -
Flags: review?(mak77)
Assignee | ||
Comment 6•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c01459669b6
Assignee | ||
Comment 7•8 years ago
|
||
(In reply to Drew Willcoxon :adw from comment #6) > https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c01459669b6 Tons of failures but they're all known and unrelated.
Comment 8•8 years ago
|
||
Comment on attachment 8722267 [details] [diff] [review] patch v2 Review of attachment 8722267 [details] [diff] [review]: ----------------------------------------------------------------- fwiw, I think when the only change is a missing QI, you should just reland without a further review ::: browser/modules/test/contentSearch.js @@ +39,5 @@ > +function waitForLoadAndStopIt(expectedURL, callback) { > + let Ci = Components.interfaces; > + let webProgress = > + docShell.QueryInterface(Ci.nsIInterfaceRequestor). > + getInterface(Ci.nsIWebProgress); nit: indent as let webProgress = docShell.QueryInterface(Ci.nsIInterfaceRequestor) .getInterface(Ci.nsIWebProgress);
Attachment #8722267 -
Flags: review?(mak77) → review+
Assignee | ||
Comment 9•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/f587e6297384
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f587e6297384
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
You need to log in
before you can comment on or make changes to this bug.
Description
•