Closed
Bug 1338215
Opened 8 years ago
Closed 8 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)
Toolkit
General
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.
|
Assignee | |
Updated•8 years ago
|
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 hidden (mozreview-request) |
|
|
Assignee | |
Comment 2•8 years ago
|
||
| mozreview-review | ||
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.
|
|
Assignee | |
Comment 3•8 years ago
|
||
Mark, Drew seems to be busy - do you have cycles to review this?
Flags: needinfo?(markh)
|
||
Comment 4•8 years ago
|
||
| mozreview-review | ||
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+
|
|
||
Comment 5•8 years ago
|
||
(In reply to :Gijs from comment #3)
> Mark, Drew seems to be busy - do you have cycles to review this?
LGTM!
Flags: needinfo?(markh)
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/68763bfa6d05
use a windowless browser for thumbnail hosting, r=markh
|
||
Comment 9•8 years ago
|
||
| bugherder | ||
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
|
|
Assignee | |
Updated•8 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•