Last Comment Bug 762993 - implement a sandbox iframe for Identity IdP provisioning
: implement a sandbox iframe for Identity IdP provisioning
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Identity (show other bugs)
: unspecified
: All All
: -- enhancement (vote)
: mozilla16
Assigned To: Matthew N. [:MattN] (behind on reviews)
:
Mentors:
Depends on: 769771 770063 817955
Blocks: 753238 771666
  Show dependency treegraph
 
Reported: 2012-06-08 11:11 PDT by Ben Adida [:benadida]
Modified: 2012-12-04 06:06 PST (History)
9 users (show)
MattN+bmo: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
+


Attachments
WIP Sandbox.jsm for Identity (4.50 KB, patch)
2012-06-11 17:44 PDT, Matthew N. [:MattN] (behind on reviews)
khuey: feedback+
Details | Diff | Review
v.1 Address feedback and added tests (19.00 KB, patch)
2012-06-20 02:04 PDT, Matthew N. [:MattN] (behind on reviews)
anant: review+
Details | Diff | Review
v.2 hiddenDOMWindow with XHTML iframe (disables dialogs) and make redirects work (20.89 KB, patch)
2012-06-28 01:57 PDT, Matthew N. [:MattN] (behind on reviews)
benadida: review+
gavin.sharp: feedback+
Details | Diff | Review
v.3 Use @mozframetype=content from bug 769771 (25.60 KB, patch)
2012-06-30 00:48 PDT, Matthew N. [:MattN] (behind on reviews)
jst: review+
Details | Diff | Review
Address comment 28 - Added test (8.87 KB, patch)
2012-06-30 22:05 PDT, Matthew N. [:MattN] (behind on reviews)
dolske: review+
Details | Diff | Review

Description Ben Adida [:benadida] 2012-06-08 11:11:55 PDT

    
Comment 1 Matthew N. [:MattN] (behind on reviews) 2012-06-11 17:44:10 PDT
Created attachment 632087 [details] [diff] [review]
WIP Sandbox.jsm for Identity

Based on Anant and Kyle's initial version but with features disabled on the docshell and stylesheets disabled.

Since we are exposing the IDP DOM APIs to all webpages, I think we don't need the Cu.Sandbox code which is commented out, correct?

Also, I looked at bug 745415 but don't see where the problem was addressed.

Is there anything else which should be done?
Comment 2 Kyle Huey [:khuey] (khuey@mozilla.com) (Away until 6/13) 2012-06-18 12:13:29 PDT
This is intended to be the iframe we load the provisioning site in, correct?
Comment 3 Ben Adida [:benadida] 2012-06-18 12:16:59 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #2)
> This is intended to be the iframe we load the provisioning site in, correct?

correct!
Comment 4 Anant Narayanan [:anant] 2012-06-18 12:21:54 PDT
Comment on attachment 632087 [details] [diff] [review]
WIP Sandbox.jsm for Identity

Review of attachment 632087 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me! Just a few nits:

::: toolkit/identity/Sandbox.jsm
@@ +41,5 @@
> +   */
> +  get id() {
> +    return this._frame.contentWindow.QueryInterface(Ci.nsIInterfaceRequestor)
> +             .getInterface(Ci.nsIDOMWindowUtils).outerWindowID;
> +  },

I'm not sure what the convention for indentation is generally, but the .getInterface on the following line looks a little off (occurs later as well).

@@ +46,5 @@
> +
> +  /**
> +   * Load or reload the url
> +   */
> +  load: function Sandbox_load() {

This function isn't used anywhere. The caller creating the sandbox provides the URL in the constructor which is then loaded by _createSandbox, also called by the constructor. What is the purpose of this function?

@@ +116,5 @@
> +         this._sandbox = new Cu.Sandbox(workerWindow, {
> +         wantXrays:        false,
> +         sandboxPrototype: workerWindow
> +         });
> +         */

Probably best to simply remove the entire block. We might need to provide the sandbox to the caller at a later point, but we don't need it for the provisioning case.

@@ +133,5 @@
> +    );
> +
> +  },
> +
> +  _log: function Sandbox__log(msg) {

Consider using log4moz at some point.
Comment 5 Kyle Huey [:khuey] (khuey@mozilla.com) (Away until 6/13) 2012-06-18 12:40:58 PDT
Comment on attachment 632087 [details] [diff] [review]
WIP Sandbox.jsm for Identity

Review of attachment 632087 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine.  Just one nit.

::: toolkit/identity/Sandbox.jsm
@@ +18,5 @@
> + * callback provided to the constructor will be invoked when the sandbox is
> + * ready to be used. The callback will receive this object as its only argument.
> + *
> + * Please call free() when you are finished with the sandbox to explicitely free
> + * up all associated resources.

Change this from "Please" to "You must".

Also, there's a typo in 'explicitly'.
Comment 6 Matthew N. [:MattN] (behind on reviews) 2012-06-20 02:04:59 PDT
Created attachment 634815 [details] [diff] [review]
v.1 Address feedback and added tests

Anant, I looked at bug 745415 but don't see where the problem was addressed; could you point me to it?

Here's a bunch of tests. test_redirect is not working at the moment. Thanks.

(In reply to Anant Narayanan [:anant] from comment #4)
> Review of attachment 632087 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good to me! Just a few nits:
> 
> ::: toolkit/identity/Sandbox.jsm
> @@ +41,5 @@
> > +   */
> > +  get id() {
> > +    return this._frame.contentWindow.QueryInterface(Ci.nsIInterfaceRequestor)
> > +             .getInterface(Ci.nsIDOMWindowUtils).outerWindowID;
> > +  },
> 
> I'm not sure what the convention for indentation is generally, but the
> .getInterface on the following line looks a little off (occurs later as
> well).

I aligned the periods now which is what I've seen in other modules.

> @@ +46,5 @@
> > +
> > +  /**
> > +   * Load or reload the url
> > +   */
> > +  load: function Sandbox_load() {
> 
> This function isn't used anywhere. The caller creating the sandbox provides
> the URL in the constructor which is then loaded by _createSandbox, also
> called by the constructor. What is the purpose of this function?

I've renamed it to "reload" to make it clearer.  It is used by the identity code when we re-use a Sandbox after the first provisioning flow fails ( PROV => AUTH => PROV ).  This is to maintain the same outer window ID for the 2 provisioning flows since that's what we key the prov. flow off of.  It would probably be better to attach some "provId" attribute or something else(?) on the frame/window in a way that nsDOMIdentity can also look it up and use it instead of the outer window ID for provisioning.  It was easier for now to just re-use the Sandbox for the 2 prov. flows and then nsDOMIdentity can always use the outer window ID.

> @@ +116,5 @@
> > +         this._sandbox = new Cu.Sandbox(workerWindow, {
> > +         wantXrays:        false,
> > +         sandboxPrototype: workerWindow
> > +         });
> > +         */
> 
> Probably best to simply remove the entire block. We might need to provide
> the sandbox to the caller at a later point, but we don't need it for the
> provisioning case.

Thanks for clarifying.  I wasn't sure if it was something I was supposed to be using.

> Consider using log4moz at some point.

If only bug 451283 would get fixed.
Comment 7 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-27 09:22:33 PDT
Comment on attachment 634815 [details] [diff] [review]
v.1 Address feedback and added tests

>diff --git a/toolkit/identity/Sandbox.jsm b/toolkit/identity/Sandbox.jsm

>+  _createFrame: function Sandbox__createFrame() {
>+    // TODO: What if there is no most recent browser window? (bug 745415).
>+    // Or use hiddenWindow

This seems like an important issue to solve, particularly if this is to be useful as a general purpose module (we might like to use this for social code as well - see bug 766607). Just using the hidden window should work fine (though you'll need to use createElementNS(XHTML_NS, "iframe"), since the hidden window document can be either xhtml (Win/Linux) or xul (Mac)).
Comment 8 Anant Narayanan [:anant] 2012-06-27 11:34:27 PDT
Comment on attachment 634815 [details] [diff] [review]
v.1 Address feedback and added tests

Review of attachment 634815 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Matthew N. [:MattN] from comment #6)
> Anant, I looked at bug 745415 but don't see where the problem was addressed;
> could you point me to it?

This fix is in this version of the patch: https://bug745345.bugzilla.mozilla.org/attachment.cgi?id=628588 - nothing fancy, just:

let aSvc = Cc["@mozilla.org/appshell/appShellService;1"].getService(Ci.nsIAppShellService);
let doc = aSvc.hiddenDOMWindow.document;

It didn't make it to the final version of the patch, because (IIRC) of the reason Gavin mentions below.

> > > +  /**
> > > +   * Load or reload the url
> > > +   */
> > > +  load: function Sandbox_load() {
> > 
> > This function isn't used anywhere. The caller creating the sandbox provides
> > the URL in the constructor which is then loaded by _createSandbox, also
> > called by the constructor. What is the purpose of this function?
> 
> I've renamed it to "reload" to make it clearer.  It is used by the identity
> code when we re-use a Sandbox after the first provisioning flow fails ( PROV
> => AUTH => PROV ).  This is to maintain the same outer window ID for the 2
> provisioning flows since that's what we key the prov. flow off of.  It would
> probably be better to attach some "provId" attribute or something else(?) on
> the frame/window in a way that nsDOMIdentity can also look it up and use it
> instead of the outer window ID for provisioning.  It was easier for now to
> just re-use the Sandbox for the 2 prov. flows and then nsDOMIdentity can
> always use the outer window ID.

Sounds good! Might help to add some comments to the function to the effect of "this makes it easier to reuse sandboxes".

> > Consider using log4moz at some point.
> 
> If only bug 451283 would get fixed.

Is adding log4moz to toolkit a prerequisite? All the services code already uses log4moz, so you could, in theory do a Cu.import("resource://services-common/log4moz.js"); no?

(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #7)
> >+  _createFrame: function Sandbox__createFrame() {
> >+    // TODO: What if there is no most recent browser window? (bug 745415).
> >+    // Or use hiddenWindow
> 
> This seems like an important issue to solve, particularly if this is to be
> useful as a general purpose module (we might like to use this for social
> code as well - see bug 766607). Just using the hidden window should work
> fine (though you'll need to use createElementNS(XHTML_NS, "iframe"), since
> the hidden window document can be either xhtml (Win/Linux) or xul (Mac)).

Ah, indeed, I think this was the issue we tripped up on. If we create a XHTML iframe, is it still possible to retrieve the docShell? I ran into some issues with permissions, but maybe I wasn't doing it right.

::: toolkit/identity/tests/chrome/Makefile.in
@@ +19,5 @@
> +		sandbox_content_popup.html \
> +		sandbox_content_redirect.html \
> +		sandbox_content_redirect.html^headers^ \
> +		sandbox_content.sjs \
> +		$(NULL)

Nice tests! \o/

::: toolkit/identity/tests/chrome/test_sandbox.xul
@@ +57,5 @@
> + * Free the sandbox and make sure all properties that are not booleans,
> + * functions or numbers were freed.
> + */
> +function free_and_check_sandbox(aSandbox) {
> +  SimpleTest.executeSoon(function() {

I think it might make more sense to put this (and the following test_creation, test_reload tests too) into an xpcshell-test. That way, when you implement the hidden DOM window that gets tested implicitly too.
Comment 9 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-27 12:01:31 PDT
Yes, you should still be able to access the docShell regardless of what kind of iframe it is, though you might need to do a slightly different funny dance?
Comment 10 Anant Narayanan [:anant] 2012-06-27 12:10:41 PDT
I tried a couple different ways, but this one is the most promising:

frame.QueryInterface(Ci.nsIInterfaceRequestor)
.getInterface(Ci.nsIWebNavigation)
.QueryInterface(Ci.nsIDocShell);

(instead of doing frame.docShell directly, which will only work if it's a XUL iframe).

Matt, can you try this and see if it works? Though we only need to do this on Linux & Windows, so you might want to add a check to find out the type of the hiddenDOMWindow first.
Comment 11 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-27 12:51:41 PDT
iframe.contentWindow.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDocShell) might be more reliable. I'm not sure how frame.QueryInterface(Ci.nsIInterfaceRequestor) works.
Comment 12 Matthew N. [:MattN] (behind on reviews) 2012-06-28 01:57:20 PDT
Created attachment 637411 [details] [diff] [review]
v.2 hiddenDOMWindow with XHTML iframe (disables dialogs) and make redirects work

Thanks for the info guys.

* 2 tests TODOs fixed: Redirects are now handled in the sandbox and dialogs from the sandbox no longer work (now that we're using the hiddenDOMWindow).
* Made test_popup cleanup better
* Switched to DOMContentLoaded instead of DOMWindowCreated so that consumers of the API (and tests) don't start probing the Sandbox private members before it's ready. This also aligns with the function name "_makeSandboxContentLoaded".

Gavin, let me know what directory to put this JSM in so it can be shared better.

(In reply to Anant Narayanan [:anant] from comment #8)
> Comment on attachment 634815 [details] [diff] [review]
> > > Consider using log4moz at some point.
> > 
> > If only bug 451283 would get fixed.
> 
> Is adding log4moz to toolkit a prerequisite? All the services code already
> uses log4moz, so you could, in theory do a
> Cu.import("resource://services-common/log4moz.js"); no?

My understanding is that toolkit shouldn't depend on Services as not all toolkit apps build services.  I've switched to using our centralized Identity logging function.

> ::: toolkit/identity/tests/chrome/test_sandbox.xul
> @@ +57,5 @@
> > + * Free the sandbox and make sure all properties that are not booleans,
> > + * functions or numbers were freed.
> > + */
> > +function free_and_check_sandbox(aSandbox) {
> > +  SimpleTest.executeSoon(function() {
> 
> I think it might make more sense to put this (and the following
> test_creation, test_reload tests too) into an xpcshell-test. That way, when
> you implement the hidden DOM window that gets tested implicitly too.

It seems like the hiddenDOMWindow doesn't exist in the xpcshell environment so unless I'm doing something wrong, they'll have to use the mochitest framework.
[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIAppShellService.hiddenDOMWindow]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/identity/Sandbox.jsm :: Sandbox__createFrame...]
Comment 13 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-28 07:57:56 PDT
Comment on attachment 637411 [details] [diff] [review]
v.2 hiddenDOMWindow with XHTML iframe (disables dialogs) and make redirects work

The _createFrame changes look good to me. Don't worry about the file location for the moment, you can land it where it is in this patch and we can always move it later.
Comment 14 Matthew N. [:MattN] (behind on reviews) 2012-06-28 15:02:07 PDT
Comment on attachment 637411 [details] [diff] [review]
v.2 hiddenDOMWindow with XHTML iframe (disables dialogs) and make redirects work

Who wants to be the toolkit peer to review this?
Comment 15 Matthew N. [:MattN] (behind on reviews) 2012-06-28 15:04:49 PDT
Comment on attachment 637411 [details] [diff] [review]
v.2 hiddenDOMWindow with XHTML iframe (disables dialogs) and make redirects work

Or perhaps this falls under the BrowserID module[1] and that page should be updated to list toolkit/identity? Then Ben can just review this.

[1] https://wiki.mozilla.org/Modules/All#BrowserID
Comment 16 Matthew N. [:MattN] (behind on reviews) 2012-06-28 15:45:56 PDT
Comment on attachment 637411 [details] [diff] [review]
v.2 hiddenDOMWindow with XHTML iframe (disables dialogs) and make redirects work

jst, can you make sure we're doing the docshell/webnavigation/presshell stuff correctly?
Comment 17 Johnny Stenback (:jst, jst@mozilla.com) 2012-06-28 21:00:58 PDT
Yup, I'm planning to spend time on this (and other identity reviews ) tomorrow.
Comment 18 Matthew N. [:MattN] (behind on reviews) 2012-06-29 03:22:37 PDT
I'm doing a try run with just this patch (removing the logging dependency on IDLog and adding 2 Makefile.in files) so we could land it first as a sanity check that we have the build system right before we push the major identity patches.

https://tbpl.mozilla.org/?tree=Try&rev=5cd08dee6740
Comment 19 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-29 09:16:02 PDT
Comment on attachment 637411 [details] [diff] [review]
v.2 hiddenDOMWindow with XHTML iframe (disables dialogs) and make redirects work

>diff --git a/toolkit/identity/Sandbox.jsm b/toolkit/identity/Sandbox.jsm

>+  _createFrame: function Sandbox__createFrame() {
>+    let hiddenWindow = Services.appShell.hiddenDOMWindow;
>+    let doc = hiddenWindow.document;
>+
>+    // Insert iframe in to create docshell.
>+    let frame = doc.createElementNS(XHTML_NS, "iframe");

Actually, I discovered in bug 762569 that this won't work - you get a chrome docshell for html iframes in the hidden document, which isn't desirable, and XHTML iframes apparently can't be marked type="content".
Comment 20 Anant Narayanan [:anant] 2012-06-29 09:23:37 PDT
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #19)
> >+  _createFrame: function Sandbox__createFrame() {
> >+    let hiddenWindow = Services.appShell.hiddenDOMWindow;
> >+    let doc = hiddenWindow.document;
> >+
> >+    // Insert iframe in to create docshell.
> >+    let frame = doc.createElementNS(XHTML_NS, "iframe");
> 
> Actually, I discovered in bug 762569 that this won't work - you get a chrome
> docshell for html iframes in the hidden document, which isn't desirable, and
> XHTML iframes apparently can't be marked type="content".

That explains the problem I was seeing.

Would the right solution then be to create a new hidden top-level XUL window if the enumerator doesn't return any, and then append an iframe to it? Hopefully the docShell still gets created even though the top level window is hidden...
Comment 21 Ben Adida [:benadida] 2012-06-29 09:29:17 PDT
Comment on attachment 637411 [details] [diff] [review]
v.2 hiddenDOMWindow with XHTML iframe (disables dialogs) and make redirects work

Review of attachment 637411 [details] [diff] [review]:
-----------------------------------------------------------------

I would prefer if Sandbox.jsm didn't depend on other identity components, e.g. IDLog, but otherwise this looks good to me from the identity point of view.
Comment 22 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-29 09:36:15 PDT
(In reply to Anant Narayanan [:anant] from comment #20)
> Would the right solution then be to create a new hidden top-level XUL window
> if the enumerator doesn't return any, and then append an iframe to it?
> Hopefully the docShell still gets created even though the top level window
> is hidden...

I'm investigating other solutions for bug 762569. One of them might be to make the default hidden window (used on Windows/Linux) be an XHTML document so that you can insert a XUL document into it.
Comment 23 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-29 13:37:27 PDT
I was wrong about the cause - the key distinction is that the document is unprivileged (can't create XUL elements), not that it's HTML. I just attached a patch in bug 769771 that should allow us to use an xhtml:iframe with mozframetype="content".
Comment 24 Ben Adida [:benadida] 2012-06-29 16:08:34 PDT
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #7)
> This seems like an important issue to solve, particularly if this is to be
> useful as a general purpose module (we might like to use this for social
> code as well - see bug 766607). Just using the hidden window should work
> fine (though you'll need to use createElementNS(XHTML_NS, "iframe"), since
> the hidden window document can be either xhtml (Win/Linux) or xul (Mac)).

Would it be okay if we reverted this change? It's causing some cascading failures, specifically in the way it interacts with frameMessageManager. I'd rather we have something working now and improve it later.
Comment 25 Matthew N. [:MattN] (behind on reviews) 2012-06-30 00:48:30 PDT
Created attachment 638086 [details] [diff] [review]
v.3 Use @mozframetype=content  from bug 769771
Comment 26 Johnny Stenback (:jst, jst@mozilla.com) 2012-06-30 00:51:39 PDT
Comment on attachment 638086 [details] [diff] [review]
v.3 Use @mozframetype=content  from bug 769771

Looks good, r=jst
Comment 27 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-30 07:10:27 PDT
(In reply to Ben Adida [:benadida] from comment #24)
> Would it be okay if we reverted this change? It's causing some cascading
> failures, specifically in the way it interacts with frameMessageManager. I'd
> rather we have something working now and improve it later.

I think, from reading later comments on IRC from Matt, that these have been sorted out?

If you are still seeing issues, I can help you figure them out.
Comment 28 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-30 07:17:34 PDT
Comment on attachment 638086 [details] [diff] [review]
v.3 Use @mozframetype=content  from bug 769771

>diff --git a/toolkit/identity/Sandbox.jsm b/toolkit/identity/Sandbox.jsm

>+  _createFrame: function Sandbox__createFrame() {

>+    frame.setAttribute("mozbrowser", true);

Is this still necessary to get the right .top behavior (or is setting .isBrowserFrame sufficient)?

The .top thing doesn't seem to be covered in the tests, can we add it?

>+  _createSandbox: function Sandbox__createSandbox(aCallback) {

>+    let webNav = this._frame.contentWindow
>+                            .QueryInterface(Ci.nsIInterfaceRequestor)
>+                            .getInterface(Ci.nsIWebNavigation);
>+    let docShell = this._frame.contentWindow
>+                              .QueryInterface(Ci.nsIInterfaceRequestor)
>+                              .getInterface(Ci.nsIWebNavigation)
>+                              .QueryInterface(Ci.nsIInterfaceRequestor)
>+                              .getInterface(Ci.nsIDocShell);
>+
>+    webNav.loadURI(
>+      this._url,
>+      docShell.LOAD_FLAGS_BYPASS_CACHE,
>+      null, // referrer
>+      null, // postData
>+      null  // headers
>+    );

You could just use Ci.nsIWebNavigation.LOAD_FLAGS_BYPASS_CACHE, and avoid doing that dance to retrieve "docShell" (which partially duplicates the dance done for webNav :).
Comment 29 Matthew N. [:MattN] (behind on reviews) 2012-06-30 22:05:27 PDT
Created attachment 638168 [details] [diff] [review]
Address comment 28 - Added test

(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #28)
> Comment on attachment 638086 [details] [diff] [review]
> v.3 Use @mozframetype=content  from bug 769771
> 
> >diff --git a/toolkit/identity/Sandbox.jsm b/toolkit/identity/Sandbox.jsm
> 
> >+  _createFrame: function Sandbox__createFrame() {
> 
> >+    frame.setAttribute("mozbrowser", true);
> 
> Is this still necessary to get the right .top behavior (or is setting
> .isBrowserFrame sufficient)?

I don't think it was necessary for the .top behaviour but there may be other behaviour fixes so I think it's safer to leave it for now. 

> The .top thing doesn't seem to be covered in the tests, can we add it?

I had the test in progress in my patch queue but didn't want to slow down the review of the code since we want to land this today. It tests:
* window.top == window
* Components.classes is not defined
* Trying to access an attribute on Ci.nsIDOMWindowUtils throws

If there are other things we should test, we can add them in a follow-up.

> You could just use Ci.nsIWebNavigation.LOAD_FLAGS_BYPASS_CACHE, and avoid
> doing that dance to retrieve "docShell" (which partially duplicates the
> dance done for webNav :).

Yes, I planned on doing that but in the rush to land this, I didn't come back to it.

One of these attributes fixed it so window.open doesn't work from the sandbox content! Another todo down :)
Comment 32 Johnny Stenback (:jst, jst@mozilla.com) 2012-07-03 14:13:27 PDT
Cc:ing jlebar here so he can look over how the mozbrowser stuff ended up being used here.
Comment 33 Justin Lebar (not reading bugmail) 2012-07-06 10:48:10 PDT
This seems relatively sane to me, although doing this means we have to lock down in-process mozbrowser properly (it currently is not bug-free).  But I'm working on it!

Note You need to log in before you can comment on or make changes to this bug.