Closed Bug 1352043 Opened 7 years ago Closed 7 years ago

Intermittent browser/components/uitour/test/browser_no_tabs.js | Test timed out -

Categories

(Firefox :: Tours, defect)

Product:
Firefox
Component:
Tours
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: Gijs)

References

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell fixed])

Attachments

(1 file)

Bug 1352043 - attempt to fix intermittents by forcing an initial system load in HiddenFrames,
59 bytes, text/x-review-board-request
kmag
: review+
Details 
All failures are window7-debug and e10s.
Logs seem to consistently look like:

13:24:28     INFO -  Buffered messages logged at 13:22:58
13:24:28     INFO -  890 INFO Entering test bound test_windowless_UITour
13:24:28     INFO -  891 INFO Adding UITour permission to the test page.
13:24:28     INFO -  Buffered messages finished
13:24:28    ERROR -  892 INFO TEST-UNEXPECTED-FAIL | browser/components/uitour/test/browser_no_tabs.js | Test timed out -
13:24:28     INFO -  GECKO(5028) | MEMORY STAT | vsize 679MB | vsizeMaxContiguous 733MB | residentFast 201MB | heapAllocated 76MB
13:24:28     INFO -  893 INFO TEST-OK | browser/components/uitour/test/browser_no_tabs.js | took 90071ms

Likely test hang after https://dxr.mozilla.org/mozilla-central/rev/b5d8b27a753725c1de41ffae2e338798f3b5cacd/browser/components/uitour/test/browser_no_tabs.js#58
Screenshot shows a "Problem loading page" tab with an info message: "Remote XUL: This page uses an unsupported technology that is no longer available by default in Firefox".

https://public-artifacts.taskcluster.net/bFCiS7-TRruEc0Q6-Rf6yA/0/public/test_info/mozilla-test-fail-screenshot_ne3upq.png
The test creates a HiddenFrame around the time of the test hang, at https://dxr.mozilla.org/mozilla-central/rev/b5d8b27a753725c1de41ffae2e338798f3b5cacd/browser/components/uitour/test/browser_no_tabs.js#19, and retriggers in comment 3 suggest this started happening shortly after bug 1351300 landed, so...

:gijs - Can you have a look?
Blocks: 1351300
Flags: needinfo?(gijskruitbosch+bugs)
Whiteboard: [stockwell needswork]
All of these have something along the lines of:

03:31:52     INFO -  GECKO(5156) | [Parent 5156] WARNING: Windowless browser was not closed prior to destruction: file z:/build/build/src/xpfe/appshell/nsAppShellService.cpp, line 470

That's probably related. It's completely not obvious to me why the browser would be destroyed, however. The caller keeps a reference to the promise, which keeps a reference to the hidden frame, which keeps a reference to the windowless browser... Kris, do you have any ideas?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(kmaglione+bmo)
Hm. Yeah, the only thing I can think of that would cause this to happen would be `createHiddenFrame` throwing before it resolved the outer promise. From the logs, that promise clearly never resolves, since we don't have the message on the first line of the resolution handler.

But I'm not sure how uitour.html could ever be loaded in that case, since the <browser> is inserted just before the resolve() call, and it shouldn't wind up loading anything if the insertion fails.

It might help to rewrite the test as something like this, so rejected promises are handled properly:

https://gist.github.com/a642b959d6cd213a4ea4cc377348df59
Flags: needinfo?(kmaglione+bmo)
I mean, the other bizarre thing here is that the data: URI that we use in the hidden frame is getting loaded in a tab in the main window (judging from both screenshots and the "extra tab" error on test cleanup). Is there something in the test that's "supposed" to do this that I'm missing (I don't see anything...)? Because otherwise that's pretty scary. Kris, I've been bugging you a lot, but do you know who knows about the implementation of windowless browsers who might know how that's happening? I don't see anything in HiddenFrame.jsm or this test that would explain that...
Flags: needinfo?(kmaglione+bmo)
OK, I have no idea how, but it looks like the load is somehow being retargeted to a tab. If this had started happening yesterday, I would have guessed bug 1338004, but alas...

So my best guess now is that we're triggering the remote XUL error when we try to assign to window.location of an unprivileged about:blank, and that's somehow being redirected to a tab.

So, taking a stab in the dark, let's just do something here that's more like what we do elsewhere:

    docShell.createAboutBlankContentViewer(systemPrincipal);
    docShell.useGlobalHistory = false;
    docShell.loadURI(XUL_PAGE, 0, null, null, null);
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(gijskruitbosch+bugs)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(gijskruitbosch+bugs)
Comment on attachment 8856540 [details]
Bug 1352043 - attempt to fix intermittents by forcing an initial system load in HiddenFrames,

https://reviewboard.mozilla.org/r/128494/#review130940

Thanks for fixing this!

::: browser/modules/HiddenFrame.jsm:82
(Diff revision 1)
>          this._frame = this._browser.document.ownerGlobal;
>          this._deferred.resolve(this._frame);
>        }
>      };
>      this._webProgress.addProgressListener(this._listener, Ci.nsIWebProgress.NOTIFY_STATE_DOCUMENT);
> -    this._browser.document.location = XUL_PAGE;
> +    let {docShell} = this._browser.document;

I always forget that `document.docShell` exists now...

So, this is fine, but it has the unfortunate side-effect of forcing an about:blank document global to be created that we're just going to throw away to replace with the chrome-privileged one. It probably doesn't matter much, but using `_browser.getInterface(Ci.nsIDocShell)` instead isn't much more complex and would avoid that issue.
Attachment #8856540 - Flags: review?(kmaglione+bmo) → review+
Keywords: leave-open
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/65f6388e03bb
attempt to fix intermittents by forcing an initial system load in HiddenFrames, r=kmag
Blocks: 1353243
No occurrences today and the only ones from yesterday were from try or before this landed/merged (on the graphics branch), so marking this fixed.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox52: --- → unaffected
status-firefox53: --- → unaffected
status-firefox54: --- → unaffected
status-firefox55: --- → fixed
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Whiteboard: [stockwell needswork] → [stockwell fixed]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: