Closed Bug 1294509 Opened 5 years ago Closed 5 years ago

Enable browser_thumbnails_bg_crash_during_capture.js on e10s

Categories

(Toolkit :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: adw, Assigned: adw)

References

Details

Attachments

(1 file)

No description provided.
Summary: Fix browser_thumbnails_bg_crash_during_capture.js → Enable browser_thumbnails_bg_crash_during_capture.js on e10s
This patch delays recreating the browser when a crash occurs.  Seems to fix the problem.

This test queues up two captures.  While the first is ongoing, the test crashes the thumbnailing browser, so then the second should immediately start and finish properly.  But what happens is that after the browser is recreated for the second capture, something's wrong with its message manager.  When BackgroundPageThumbs sends the capture message to it, sendAsyncMessage throws with NS_ERROR_NOT_INITIALIZED.  That's where the test fails.  But it doesn't always happen, so the test is intermittent.  And this is only when e10s is enabled.

My guess is that it's not safe to try and respawn the content process in crash listeners because the OOP code just isn't ready to handle that yet or something.
Comment on attachment 8780293 [details]
Bug 1294509 - Enable browser_thumbnails_bg_crash_during_capture.js on e10s.

https://reviewboard.mozilla.org/r/70998/#review68582

Looks fine to me (with or without the suggestion below)

::: toolkit/components/thumbnails/BackgroundPageThumbs.jsm:239
(Diff revision 1)
> +        // immediately (on the same stack as the _done call stack) if there are
> +        // any more queued-up captures, and that seems to mess up the new
> +        // browser's message manager if it happens on the same stack as the
> +        // listener.  Trying to send a message to the manager in that case
> +        // throws NS_ERROR_NOT_INITIALIZED.
> +        let timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);

I'd have thought Services.tm.currentThread.dispatch would be a little cleaner here and avoid the explicit strong reference? I don't really care though :)
Attachment #8780293 - Flags: review?(markh) → review+
Ah yeah, thanks!
https://hg.mozilla.org/integration/fx-team/rev/6f2ba7787816fd01f77c30e3bdb68400db74bf0c
Bug 1294509 - Enable browser_thumbnails_bg_crash_during_capture.js on e10s. r=markh
https://hg.mozilla.org/mozilla-central/rev/6f2ba7787816
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.