537428 - Figure out a better "need a new rendering area" API for nsIBrowserDOMWindow

Status: NEW

(Core :: DOM: Navigation, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Right now, the way core code (in particular nsContentTreeOwner) handles implementing nsIWindowProvider is as follows:

1)  Examine the browser.link.open_newwindow pref.
2)  If it's not set to OPEN_NEWTAB or OPEN_CURRENTWINDOW, just fall back on the
    window creator.
3)  If the browser.link.open_newwindow.restriction pref doesn't allow following
    the pref above, fall back on the window creator.
4)  Call OpenURI on nsIBrowserDOMWindow with OPEN_NEW as the context and either
    OPEN_NEWTAB or OPEN_CURRENTWINDOW as aWhere.
5)  Decide based on how we called it whether the window we're getting back is
    "new".

In the e10s world, we don't want to be passing around content windows to the chrome process, we don't want to be identifying rendering areas by nsIDOMWindow pointers in the chrome process, and we can't be creating new rendering areas in the content process.  In bug 516747 I put things together for now such that we just always open a new tab whenever we need a "new window"; for the Fennec testbed this is OK.

In general, though, should the above 5-step process live in core code or in the nsIBrowserDOMWindow implementation?  What should the nsIBrowserDOMWindow API it uses look like?

What core code has at the point when we're talking about targeting a "new window" load is whatever is passed to nsIWindowProvider (though we can add things here as needed).

As a separate question, there's the issue of what should happen with modal content windows, alerts and the like.  Perhaps we should actually make sure that we call into the windowwatcher in the _parent_ process only, and add new windowwatcher API to that effect...

In any case, any thoughts here are much appreciated.

Comment 1

[[e-mail]]

So, the current API is used in the following circumstances:
1. Open of an external URI e.g. via -remote. There is no opener, the context is always external, and the where is usually default but can be new tab or window.
2. Creation of a content area for a link target or a window.open call. The URI is no longer provided, the context is always internal, and the where is always current or new tab. The return value is the tab in question, which in the case of the current tab is also the top content window of the opener window.

As most of the link target or window open behaviour seems to have already been implemented in the content tree owner and the only use of the API is to create a tab where necessary, I suggest that you add a new method to do just that.

Comment 2

[Boris Zbarsky [:bzbarsky]]

e10s doesn't and won't (and can't easily) use the content tree owner; sorry for not making that clear.

I agree about your two sets of circumstances; I question the exact division of #2 between browser UI code and Gecko C++ code.  And in particular, I'm questioning whether TabChild/TabParent need to duplicate the content tree owner pref stuff, or whether that would better be left to the XUL app.

Comment 3

[[e-mail]]

Regarding the division of labour between C++ and UI, I don't think that the C++ needs to read the prefs; it currently does that but only to avoid asking the UI for a new window, which is hard from the UI point of view as it needs to spin the event loop waiting for the new window's tabbrowser to be created.

So in pseudocode, I think it should look like this:
1. Look for tabbrowser support (equivalent of current nsIBrowserDOMWindow)
2. If found, ask the tabbrowser support for a browser element.
3. If a browser element was returned, open the request into its TabChild.
4. Otherwise, create a new window in C++.

Comment 4

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

(In reply to comment #0)
> As a separate question, there's the issue of what should happen with modal
> content windows, alerts and the like.  Perhaps we should actually make sure
> that we call into the windowwatcher in the _parent_ process only, and add new
> windowwatcher API to that effect...

Something in content process just implements nsIWindowCreator2 and forwards
calls to chrome process, which then calls its windowwatcher.
Since modal *windows* aren't available to content scripts, I don't think they
really ever need to access non-chrome-opener.

I'm probably going to try the nsIWindowCreator2 approach. It should be
pretty easy to implement.

Comment 5

[Johnny Stenback (:jst)]

(In reply to comment #4)
> Since modal *windows* aren't available to content scripts, I don't think they
> really ever need to access non-chrome-opener.

It's not clear to me that that's always the case, in particular with window.showModalDialog()

Comment 6

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

Indeed. I and bz discussed about this on IRC, and showModalDialog shouldn't cause
any problems. Whatever is loaded in it will use the same process as the opener.