-
Notifications
You must be signed in to change notification settings - Fork 259
Bug 820953 - Expose DOM window to the add-on #698
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| /* 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/. | ||
| */ | ||
|
|
||
| "use strict"; | ||
|
|
||
| module.metadata = { | ||
| "stability": "experimental" | ||
| }; | ||
|
|
||
| const { backgroundify, make, getDOMWindow } = require("../window/utils"); | ||
| const { defer } = require("../core/promise"); | ||
| const { when: unload } = require("../system/unload"); | ||
|
|
||
| // Once Bug 565388 is fixed and shipped we'll be able to make invisible, | ||
| // permanent docShells. Meanwhile we create hidden top level window and | ||
| // use it's docShell. Also note that we keep a reference to xulWindow since | ||
| // otherwise it's unloaded, on the other hand this will require no cleanup | ||
| // from our side since once add-on is unloaded window will be removed | ||
| // automatically. | ||
| const xulWindow = backgroundify(make()) | ||
| const docShell = xulWindow.docShell; | ||
|
|
||
| // Get a reference to the DOM window of the given docShell and load | ||
| // such document into that would allow us to create XUL iframes, that | ||
| // are necessary for hidden frames etc.. | ||
| const window = docShell.contentViewer.DOMDocument.defaultView; | ||
| window.location = "data:application/xhtml+xml;charset=utf-8,<html/>"; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a good url to use? Can things like websockets etc. work from data: urls?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's URL that bz suggested us to use for this. |
||
|
|
||
| // Create a promise that is delivered once add-on window is interactive, | ||
| // used by add-on runner to defer add-on loading until window is ready. | ||
| let { promise, resolve } = defer(); | ||
| window.addEventListener("DOMContentLoaded", function handler(event) { | ||
| window.removeEventListener("DOMContentLoaded", handler, false); | ||
| resolve(); | ||
| }, false); | ||
|
|
||
| exports.ready = promise; | ||
| exports.window = window; | ||
|
|
||
| // Still close window on unload to claim memory back early. | ||
| unload(function() { window.close() }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,9 +13,7 @@ exports['test loader'] = function(assert) { | |
| prints.push(message); | ||
| } | ||
|
|
||
| let options = JSON.parse(JSON.stringify(packaging)); | ||
|
|
||
| let loader = Loader(override(options, { | ||
| let loader = Loader(override(packaging, { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No it was done just to copy options from the loader. This no longer works because with this addition: it no longer JSON since window can not be serialized. |
||
| globals: { | ||
| print: print, | ||
| foo: 1 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to load the
addon/windowat load time regardless if it is used or not, am I mistaken?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am concerned about this too. Do we have any information on timing and memory penalties that this incurs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do use windows regardless of this change, in hidden-window any API doing some ui depends on implicitly. The problem is that all of that APIs need to wait on their own and deal with synchronicity & only on some platforms.
I don't think it's worth it, I'd rather delay once and make all APIs be able to do work synchronously that curry the burden of platfrom specefic inconsistencies into all modules. Not to mention that this that load is still delayed by async read of l10n data etc..
Also note that this is temporary workaround that will go away once platform fix for Bug 565388 will be implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we open the window before we wait for APP_STARTUP so add-ons start as fast as possible on startup?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably could but intention of delaying was to avoid slowing down a startup. We could also start loading l10n stuff before startup, but we decided not to for the same reasons. We can revisit these decisions, but not sure we need to block this patch for that. We can change time we start loading when we want to.