755001 - Add "remote" attribute to <iframe mozbrowser>

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Justin Lebar (not reading bugmail)]

We have OOP mozbrowser, but unfortunately we can't turn it on everywhere because some apps use APIs which don't work OOP (e.g. indexeddb).

cjones would like us to be able to selectively turn on OOP mozbrowser so we can roll it out gradually.

Comment 1

[Justin Lebar (not reading bugmail)]

Attachment: Patch v1

No tests because whether a frame is OOP is intentionally difficult to detect!  I tested this manually.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

I don't understand this.
We don't know beforehand if the page uses indexeddb.

What is the reason for this bug?

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Olli Pettay [:smaug] from comment #2)
> I don't understand this.
> We don't know beforehand if the page uses indexeddb.
> 
> What is the reason for this bug?

I assume this is for prototyping. We know in advance in Gaia so it will be possible to start using OOP iframe for some apps and debug possible problems while the work for OOP indexedDB is ongoing.

Comment 4

[Justin Lebar (not reading bugmail)]

Exactly what Vivien said.  :)

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 623824 [details] [diff] [review]
Patch v1

Ok, but please fix the other bug I just filed ;)

Comment 6

[Justin Lebar (not reading bugmail)]

Building now; I'll post the patch once I've tested it.  :)

Comment 7

[Ed Morley [:emorley]]

Sorry had to back the push out for failures in test_browserFrame7.html:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=f177646e2aa2

{
3390 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/browser-frame/test_browserFrame7.html | top [object Window] != [object Window]
3391 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/browser-frame/test_browserFrame7.html | parent [object Window] != [object Window]
3392 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/browser-frame/test_browserFrame7.html | frameElement [object HTMLIFrameElement] != null
3393 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/browser-frame/test_browserFrame7.html | inner top [object Window] != [object Window]
}

https://hg.mozilla.org/integration/mozilla-inbound/rev/d124a9678e93

Comment 8

[Justin Lebar (not reading bugmail)]

I broke my own test!  We were running everything in-process (in the tests), and browserFrame7 fails in-process.

Comment 9

[Justin Lebar (not reading bugmail)]

Attachment: Part 2: Test fixes.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Please follow the tree rules when landing on inbound (posting a changeset link, setting target milestone, etc.)

http://hg.mozilla.org/integration/mozilla-inbound/rev/34778b6d15ed
https://hg.mozilla.org/mozilla-central/rev/34778b6d15ed

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to Justin Lebar [:jlebar] from comment #0)
> We have OOP mozbrowser, but unfortunately we can't turn it on everywhere
> because some apps use APIs which don't work OOP (e.g. indexeddb).
> 
> cjones would like us to be able to selectively turn on OOP mozbrowser so we
> can roll it out gradually.

Whoa!  Pref-able is fine!  We don't want this decision under the control of web content.

I suggested making an <iframe mozapp> that simply mapped to remote=false for now, while leaving <iframe mozbrowser> mapped to whatever the pref says.

Can we do that in a followup?

Comment 12

[Justin Lebar (not reading bugmail)]

I'm sorry I misunderstood you.

It would be nice to be able to run some apps OOP right now -- for example, we'd want to test OOP browser running OOP tabs.  So we could map mozbrowser to a pref and expose remote=? on mozapp, if you wanted.   mozbrowser is a more content-y thing than mozapp.

But I thought this whole thing was a temporary hack, and that everything was going to be OOP ASAP, so I think we may have better things to do atm than mess with this further.

Comment 13

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to Justin Lebar [:jlebar] from comment #12)
> I'm sorry I misunderstood you.
> 

No worries!

> It would be nice to be able to run some apps OOP right now -- for example,
> we'd want to test OOP browser running OOP tabs.

We could do that by loading the browser app in an <iframe mozbrowser> with the OOP pref flipped on, no?

> But I thought this whole thing was a temporary hack, and that everything was
> going to be OOP ASAP, so I think we may have better things to do atm than
> mess with this further.

Temporary hack, fine, but we need to remove this.  Is there a bug filed?

Comment 14

[Justin Lebar (not reading bugmail)]

> We could do that by loading the browser app in an <iframe mozbrowser> with the OOP pref flipped on, 
> no?

We could, except in the days since you've been gone, Mounir has been doing work on mozapp to make it actually different than mozbrowser.  The current state as of patches I've reviewed is that mozapps have manifests but we don't do anything with them.

But I don't think we can rely on being able to substitute mozbrowser for mozapp for much longer, if it even works now.

I have a bad track record with filing bugs to get around to sometime later, but I'll file a bug on backing this out and see if I have more success here.

Comment 15

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to Justin Lebar [:jlebar] from comment #14)
> > We could do that by loading the browser app in an <iframe mozbrowser> with the OOP pref flipped on, 
> > no?
> 
> We could, except in the days since you've been gone, Mounir has been doing
> work on mozapp to make it actually different than mozbrowser.

Huzzah!

> But I don't think we can rely on being able to substitute mozbrowser for
> mozapp for much longer, if it even works now.
> 

Absolutely.  When we get to that point, we should add an out-of-process mozapp pref.