Closed Bug 1338215 Opened 7 years ago Closed 7 years ago

Thumbnail code should be using a windowless browser and its own document instead of using the hidden window and chrome://...mozilla.xhtml

Categories

(Toolkit :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

Attachments

(1 file)

The hidden window is... hacky. We now have a supported way of creating invisible windows - createWindowlessBrowser() (on the appshell service) - that creates nsIWindowlessBrowser instances that we can use for snapshots. We should swap over the background thumbnail service to it. While we're here, we might as well start using a different hosting document to aboutMozilla. I'd use about:blank created with the system principal except I kind of want to stop that being possible, too, so a separate document seems best. Plus, that means it can actually show up with a meaningful URL in e.g. about:memory reports.
Summary: Thumbnail code should be using a windowless browser and its own document instead of using the hidden window and chrome://...aboutMozilla. → Thumbnail code should be using a windowless browser and its own document instead of using the hidden window and chrome://...mozilla.xhtml
Comment on attachment 8835986 [details]
Bug 1338215 - use a windowless browser for thumbnail hosting,

https://reviewboard.mozilla.org/r/111514/#review112832

::: toolkit/components/thumbnails/content/backgroundPageThumbsContent.js:190
(Diff revision 1)
> +    // It's possible we've been destroyed by now, if so don't do anything:
> +    if (!docShell) {
> +      return;
> +    }

I spotted uncaught exceptions from the loadURI call here (when running automated tests) because the `this._webNav` getter broke because `docShell` was null/undefined. I'm guessing if the remote browser is unloaded and this is called 'late', this can happen, so I added a simple guard to silence the warnings.
Mark, Drew seems to be busy - do you have cycles to review this?
Flags: needinfo?(markh)
Comment on attachment 8835986 [details]
Bug 1338215 - use a windowless browser for thumbnail hosting,

https://reviewboard.mozilla.org/r/111514/#review116190

::: toolkit/components/thumbnails/content/backgroundPageThumbs.xhtml:7
(Diff revision 1)
> +
> +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +
> +<html xmlns="http://www.w3.org/1999/xhtml">

I think a comment in this file indicating why it exists and that it's always loaded via chrome:// might be helpful
Attachment #8835986 - Flags: review+
(In reply to :Gijs from comment #3)
> Mark, Drew seems to be busy - do you have cycles to review this?

LGTM!
Flags: needinfo?(markh)
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/68763bfa6d05
use a windowless browser for thumbnail hosting, r=markh
https://hg.mozilla.org/mozilla-central/rev/68763bfa6d05
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Depends on: 1314855
Blocks: 1145470
See Also: → 1352545
You need to log in before you can comment on or make changes to this bug.