Closed Bug 811344 Opened 12 years ago Closed 4 years ago

Add ability to create <iframe mozbrowser> which uses specified data jar

Categories

(Core :: DOM: Core & HTML, defect, P5)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
RESOLVED INVALID

People

(Reporter: sicking, Unassigned)

References

Details

There's currently two ways of using <iframe mozbrowser>:

* <iframe mozbrowser> creates a browsing context which as storage-data-jar uses
  the appid of the parent browsing context, but with the browser-flag set to
  true.
* <iframe mozbrowser mozapp="manifesturl"> creates a browsing context which as
  storage-data-jar uses the appid of the specified manifesturl and with the
  browser-flag set to false.

We need a third way which allows creating a browsing context which uses a storage-data-jar of a specified appid and with the browser-flag set to *true*.

I'm sure Justin will ask what the security-data-jar should be, and I think the answer is that it should be the same as the storage-data-jar, though I'd like to double-check what the question actually means since I'm still not sure we're aligned on storage vs. security jars.
Why not just put the mozbrowser frame inside an app frame?

  <iframe mozbrowser mozapp="manifesturl" src="wrapper.html?target=page_to_load.html remote=true>

which loads

  <iframe mozbrowser src="page_to_load.html>
Because then we have to implement the whole browser API inside that app. I.e. that app would have to respond to all events fired by the inner <iframe>. We already have an implementation for those event handlers in the system app and so the gaia team wanted to leverage that implementation.
cc'ing Vivien and Ben who might be able to provide a better answer to comment 1.

One problem that we would have to solve is that we need to tell the contents of the outer iframe, which url to open in the inner iframe. But I think that might be doable by having the system app do something like:

<iframe mozbrowser mozapp="manifesturl" src="app://.../index.html?url=url-goes-here">
I'm not convinced that is a compelling reason to ask for a complex platform change at this stage of the game.

I was under the impression that the Gaia folks are able to share js files between apps, so maybe that could help them.

Note that if we did

  <iframe mozbrowser mozapp remote=false>
    <iframe mozbrowser remote=true>

then the system app /could/ (under our current implementation of mozapp) do whatever it wanted to the inner iframe, including add event listeners and so on, presuming that it was same-origin.  I suppose that would mean that we'd need to allow multiple origins per app; do we have that?  Or maybe there's a window.origin trick we can play here.

In any case, we can't keep making risky platform changes to meet ever-changing specs for e.me, and in particular I'm not wild about making a complex platform change at this point to meet a purely ergonomic issue.
We don't have multiple apps per origin no. Changing that would be pretty hard (but not impossible) at this stage, though the regression risk is pretty high since the effects would spread pretty far. The problem is that we use the origin as app-identifying-key in a number of places.

Note that this isn't just for everything.me, but also for bookmarks saved to the homescreen. The goal here is to move more things out of the homescreen process, which may or may not be a blocker (just noticed that I forgot to cc cjones, so doing so now).

I'll let the gaia team answer the question of possibility of the alternatively proposed solution.

I had hoped that this would be a pretty simple platform change, but it doesn't sound like that is the case. So we should evaluate alternative solutions here.
(In reply to Justin Lebar [:jlebar] from comment #4)
> I'm not convinced that is a compelling reason to ask for a complex platform
> change at this stage of the game.
> 

Can you explain the complexity in a bit more detail?
(In reply to Justin Lebar [:jlebar] from comment #4)
> I was under the impression that the Gaia folks are able to share js files
> between apps, so maybe that could help them.

What do you mean by this and how would that help? The only way JavaScript files are shared between apps at the moment is to copy them from a central location at build time to each individual app. Are you suggesting that we should do this to duplicate the implementation of alert/confirm/prompt/http auth/permissions dialogs etc. from the system app to another app?
> Are you suggesting that we should do this to duplicate the implementation of alert/confirm
> /prompt/http auth/permissions dialogs etc. from the system app to another app?

Yes, that was what I was hoping you could do.  Note that's not the only possible workaround here.

> Can you explain the complexity in a bit more detail?

The last time I rewrote this code, it took two weeks.  How long do you expect this to take me?  Three days?  That's three days I could have spent optimizing memory usage.  

We're way, way, way past platform feature-freeze.  I understand that this is painful for Gaia developers, but I don't think "I don't want to do it that way" is a compelling reason to so flagrantly violate our own rules (which exist specifically to avoid this situation!).
(In reply to Justin Lebar [:jlebar] from comment #8)
> > Can you explain the complexity in a bit more detail?
> 
> The last time I rewrote this code, it took two weeks.  How long do you
> expect this to take me?  Three days?  That's three days I could have spent
> optimizing memory usage.  

Would the change here require another rewrite?
> Would the change here require another rewrite?

It's hard to say.  I'd estimate it would take me three days, but it could be more or less time.  I don't think it requires a full rewrite like the last change did.

(FIWW, I'd probably try to make <iframe mozapp> be a "plain app" and make <iframe mozbrowser mozapp> be the thing you're asking for here.)

Again, I think we should consider what we're trading off here.  There's a tendency to respond to people who complain loudly about a pain point, but fixing their pain is not free.  We still have major memory issues in the platform, and I'm one of the few people trying to work full-time on them.  <sunk cost fallacy>We already lost two weeks of my time due to the last rewrite of this stuff.</sunk cost fallacy>

It's very hard for me to see the value in fixing a bug which we all agree can be worked around with no ill effects on users, when that means less time to spend on bugs which actively harm the user experience.  The whole point of our schedule is to force us to accept that we won't fix every platform bug which makes Gaia's life difficult but not impossible.  I can't point to any specific memory bug which won't get fixed if I work on this, but that doesn't mean there won't be an effect.

On the other hand, continuing to justify myself here is less productive than fixing either bug.  I'm happy to do whatever sicking + cjones would like, rather than continue to debate this.  I've said my piece.  :)
(In reply to Justin Lebar [:jlebar] from comment #4)
> I'm not convinced that is a compelling reason to ask for a complex platform
> change at this stage of the game.
> 
> I was under the impression that the Gaia folks are able to share js files
> between apps, so maybe that could help them.

Copy-Pasting mostly. Hopefully at build time.

> 
> Note that if we did
> 
>   <iframe mozbrowser mozapp remote=false>
>     <iframe mozbrowser remote=true>
> 
> then the system app /could/ (under our current implementation of mozapp) do
> whatever it wanted to the inner iframe, including add event listeners and so
> on, presuming that it was same-origin.  I suppose that would mean that we'd
> need to allow multiple origins per app; do we have that?  Or maybe there's a
> window.origin trick we can play here.
> 

Does it means all e.me apps will run in the same process? I thought one of the expectation of the platform change would be to let each of them runs into their own process.

Or is the question I'm asking here is out of the scope of this bug and needs an other one?
> Does it means all e.me apps will run in the same process?

If and only if you specify the same app manifest (mozapp=X) for every e.me app.  That's consistent across my proposal/workaround here and Jonas's proposal.
(In reply to Justin Lebar [:jlebar] from comment #8)
> We're way, way, way past platform feature-freeze.  I understand that this is
> painful for Gaia developers, but I don't think "I don't want to do it that
> way" is a compelling reason to so flagrantly violate our own rules (which
> exist specifically to avoid this situation!).

FWIW, I'm not sure that "we're past platform feature freeze" is a very compelling reason here. We should chose to solve this in platform vs. gaia based on which solution takes more time and how resource constrained the various parties are.

(In reply to Justin Lebar [:jlebar] from comment #12)
> > Does it means all e.me apps will run in the same process?
> 
> If and only if you specify the same app manifest (mozapp=X) for every e.me
> app.  That's consistent across my proposal/workaround here and Jonas's
> proposal.

This is a bit of a problem, and weakens all of the proposals here :( I had assumed that each new remote=true <iframe mozbrowser> would create a new process, but it makes sense that we reuse processes that share the same appid.

One solution would be to introduce an attribute-value like remote=noreuse which would always create a new process. That would be orthogonal to the other discussions here I'd think.
> One solution would be to introduce an attribute-value like remote=noreuse which would 
> always create a new process.

That further complicates the matter with respect to window.open -- the opened frame must be in the same process as the opened frame, so we'd have to somehow ignore this attribute on opened frames, or something...

We could just as easily have <iframe mozapp=X mozappextra=Y>, and you get placed into the same process iff you match X and Y.

But do you want all e.me apps to share the same data and security jars?
(In reply to Justin Lebar [:jlebar] from comment #14)
> > One solution would be to introduce an attribute-value like remote=noreuse which would 
> > always create a new process.
> 
> That further complicates the matter with respect to window.open -- the
> opened frame must be in the same process as the opened frame, so we'd have
> to somehow ignore this attribute on opened frames, or something...

Would it be simpler if we added the ability to say window.open(url, name, "...,remote=yes,...")? Gecko would then immediately know not to return a window object for such a call.

Just so I understand the internals better, what happens currently when window.open(...) is called? Does the child create a window object and which is immediately return, and we then, asynchronously in the parent window return a <iframe> object which has to be immediately inserted?

Or do we block the child until the parent has received the <iframe> and had a chance to insert it into a document? And only after that do we create a window object and return from the window.open call?

To put it another way, in which order do the following things happen when window.open is called:
* A window object for the child is created.
* window.open returns.
* An event is fired in the parent with a provided <iframe>.
* The <iframe> is inserted into the document.

Also, what happens if the <iframe> isn't inserted into the document?

> We could just as easily have <iframe mozapp=X mozappextra=Y>, and you get
> placed into the same process iff you match X and Y.

I don't understand. If the system app can set some attribute on the iframe element which causes the app to run in a different process, why is it easier if that attribute is a mozappextra=Y attribute vs. a remote=noreuse attribute?

> But do you want all e.me apps to share the same data and security jars?

We do want them to run in the same data jar I believe. Or rather, we don't have a choice since we don't have the ability to dynamically generate apps.

But we want homescreen bookmarks to run in separate apps so that when one runs out of memory they don't all die. I suspect homescreen bookmarks are more likely than other "apps" to run out of memory since they are more likely to be written for desktop.

However all of our storage APIs are totally fine with having multiple processes use the same data jar.
> If the system app can set some attribute on the iframe element which causes the app to run in a 
> different process, why is it easier if that attribute is a mozappextra=Y attribute vs. a 
> remote=noreuse attribute?

When we do window.open, we want the opened window to run in the same process as the opener window.  It would be good if that was reflected in the iframes' attributes.  If you have two iframes with remote=noreuse, or one with remote=noreuse and one without it, you might reasonably conclude that they don't run in the same process, even though, if one opened the other via window.open, they do.

> Would it be simpler if we added the ability to say window.open(url, name, "...,remote=yes,...")? 
> Gecko would then immediately know not to return a window object for such a call.

I don't think this is significantly simpler; the window.open flow is pretty complex.  And anyway, we want a declarative way to create these frames.

> Or do we block the child until the parent has received the <iframe> and had a chance to insert it 
> into a document? And only after that do we create a window object and return from the window.open 
> call?

We do this.  One of the reasons is that we want to give the parent a chance to block the popup.  It does so by not inserting the iframe into the DOM.
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.
Priority: -- → P5
Component: DOM → DOM: Core & HTML

We removed mozbrowser.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.