Don't allow main world initialization while creating the initial empty document

Don't allow main world initialization while creating the initial empty document

In addition to possibly causing problems in blink, this seems to be responsible
for an extensions bug where script contexts would be mis-classified (because the
context was created before the url was set).

BUG=340382

[Devlin, 2015-11-09 17:07:01]

Description was changed from

==========
Testing
BUG=
==========

to

==========
Don't allow main world initialization while creating the initial empty document

In addition to possibly causing problems in blink, this seems to be responsible
for an extensions bug where script contexts would be mis-classified (because the
context was created before the url was set).

BUG=340382
==========

[Devlin, 2015-11-09 18:11:10]

Patchset #2 (id:20001) has been deleted

[Devlin, 2015-11-09 19:49:01]

rdevlin.cronin@chromium.org changed reviewers:
+ haraken@chromium.org, japhet@chromium.org

[Devlin, 2015-11-09 19:49:01]

haraken@, mind taking a look at the blink change?  The CL description and bug
have some background, but basically extensions code was freaking out because we
got a didCreateScriptContext notification without an associated url.  The
blink-side change was the recommendation of japhet@, and fixes it.

[haraken, 2015-11-09 20:03:28]

On 2015/11/09 19:49:01, Devlin wrote:
> haraken@, mind taking a look at the blink change?  The CL description and bug
> have some background, but basically extensions code was freaking out because
we
> got a didCreateScriptContext notification without an associated url.  The
> blink-side change was the recommendation of japhet@, and fixes it.

Actually I'm not quite sure if this is a right fix. The current Blink is
assuming that ScriptController::initializeMainWorld unconditionally initializes
the main world if it's not yet initialized. This CL is going to break the
assumption.

[Devlin, 2015-11-09 21:02:24]

On 2015/11/09 20:03:28, haraken wrote:
> On 2015/11/09 19:49:01, Devlin wrote:
> > haraken@, mind taking a look at the blink change?  The CL description and
bug
> > have some background, but basically extensions code was freaking out because
> we
> > got a didCreateScriptContext notification without an associated url.  The
> > blink-side change was the recommendation of japhet@, and fixes it.
> 
> Actually I'm not quite sure if this is a right fix. The current Blink is
> assuming that ScriptController::initializeMainWorld unconditionally
initializes
> the main world if it's not yet initialized. This CL is going to break the
> assumption.

Okay.  Can you think of an alternative way to ensure that when
didCreateScriptContext is called there is an associated URL (or an alternative
for accessing the url during that notification, or something to wait for
following that)?

[haraken, 2015-11-09 22:47:11]

On 2015/11/09 21:02:24, Devlin wrote:
> On 2015/11/09 20:03:28, haraken wrote:
> > On 2015/11/09 19:49:01, Devlin wrote:
> > > haraken@, mind taking a look at the blink change?  The CL description and
> bug
> > > have some background, but basically extensions code was freaking out
because
> > we
> > > got a didCreateScriptContext notification without an associated url.  The
> > > blink-side change was the recommendation of japhet@, and fixes it.
> > 
> > Actually I'm not quite sure if this is a right fix. The current Blink is
> > assuming that ScriptController::initializeMainWorld unconditionally
> initializes
> > the main world if it's not yet initialized. This CL is going to break the
> > assumption.
> 
> Okay.  Can you think of an alternative way to ensure that when
> didCreateScriptContext is called there is an associated URL (or an alternative
> for accessing the url during that notification, or something to wait for
> following that)?

I'm not familiar with extensions, but it seems that you need to implement a way
to get an url when the url becomes available. It seems that
didCreateScriptContext is too early to get the url.

[Devlin, 2015-11-09 22:49:35]

On 2015/11/09 22:47:11, haraken wrote:
> On 2015/11/09 21:02:24, Devlin wrote:
> > On 2015/11/09 20:03:28, haraken wrote:
> > > On 2015/11/09 19:49:01, Devlin wrote:
> > > > haraken@, mind taking a look at the blink change?  The CL description
and
> > bug
> > > > have some background, but basically extensions code was freaking out
> because
> > > we
> > > > got a didCreateScriptContext notification without an associated url. 
The
> > > > blink-side change was the recommendation of japhet@, and fixes it.
> > > 
> > > Actually I'm not quite sure if this is a right fix. The current Blink is
> > > assuming that ScriptController::initializeMainWorld unconditionally
> > initializes
> > > the main world if it's not yet initialized. This CL is going to break the
> > > assumption.
> > 
> > Okay.  Can you think of an alternative way to ensure that when
> > didCreateScriptContext is called there is an associated URL (or an
alternative
> > for accessing the url during that notification, or something to wait for
> > following that)?
> 
> I'm not familiar with extensions, but it seems that you need to implement a
way
> to get an url when the url becomes available. It seems that
> didCreateScriptContext is too early to get the url.

I'm unfamiliar with blink innards (together, we have all the answers! ;)).  Is
there a WebFrame notification that corresponds with the url becoming available?

[haraken, 2015-11-09 22:50:43]

haraken@chromium.org changed reviewers:
+ dcheng@chromium.org

[haraken, 2015-11-09 22:50:43]

+dcheng

[Nate Chapin, 2015-11-09 22:51:17]

On 2015/11/09 22:49:35, Devlin wrote:
> On 2015/11/09 22:47:11, haraken wrote:
> > On 2015/11/09 21:02:24, Devlin wrote:
> > > On 2015/11/09 20:03:28, haraken wrote:
> > > > On 2015/11/09 19:49:01, Devlin wrote:
> > > > > haraken@, mind taking a look at the blink change?  The CL description
> and
> > > bug
> > > > > have some background, but basically extensions code was freaking out
> > because
> > > > we
> > > > > got a didCreateScriptContext notification without an associated url. 
> The
> > > > > blink-side change was the recommendation of japhet@, and fixes it.
> > > > 
> > > > Actually I'm not quite sure if this is a right fix. The current Blink is
> > > > assuming that ScriptController::initializeMainWorld unconditionally
> > > initializes
> > > > the main world if it's not yet initialized. This CL is going to break
the
> > > > assumption.
> > > 
> > > Okay.  Can you think of an alternative way to ensure that when
> > > didCreateScriptContext is called there is an associated URL (or an
> alternative
> > > for accessing the url during that notification, or something to wait for
> > > following that)?
> > 
> > I'm not familiar with extensions, but it seems that you need to implement a
> way
> > to get an url when the url becomes available. It seems that
> > didCreateScriptContext is too early to get the url.
> 
> I'm unfamiliar with blink innards (together, we have all the answers! ;)).  Is
> there a WebFrame notification that corresponds with the url becoming
available?

didCommitProvisionalLoad is approximately that.

[dcheng, 2015-11-10 00:25:51]

On 2015/11/09 at 22:51:17, japhet wrote:
> On 2015/11/09 22:49:35, Devlin wrote:
> > On 2015/11/09 22:47:11, haraken wrote:
> > > On 2015/11/09 21:02:24, Devlin wrote:
> > > > On 2015/11/09 20:03:28, haraken wrote:
> > > > > On 2015/11/09 19:49:01, Devlin wrote:
> > > > > > haraken@, mind taking a look at the blink change?  The CL
description
> > and
> > > > bug
> > > > > > have some background, but basically extensions code was freaking out
> > > because
> > > > > we
> > > > > > got a didCreateScriptContext notification without an associated url.

> > The
> > > > > > blink-side change was the recommendation of japhet@, and fixes it.
> > > > > 
> > > > > Actually I'm not quite sure if this is a right fix. The current Blink
is
> > > > > assuming that ScriptController::initializeMainWorld unconditionally
> > > > initializes
> > > > > the main world if it's not yet initialized. This CL is going to break
the
> > > > > assumption.
> > > > 
> > > > Okay.  Can you think of an alternative way to ensure that when
> > > > didCreateScriptContext is called there is an associated URL (or an
> > alternative
> > > > for accessing the url during that notification, or something to wait for
> > > > following that)?
> > > 
> > > I'm not familiar with extensions, but it seems that you need to implement
a
> > way
> > > to get an url when the url becomes available. It seems that
> > > didCreateScriptContext is too early to get the url.
> > 
> > I'm unfamiliar with blink innards (together, we have all the answers! ;)). 
Is
> > there a WebFrame notification that corresponds with the url becoming
available?
> 
> didCommitProvisionalLoad is approximately that.

At some point, I think we need to figure out how all these different
WebFrameClient callbacks (didCommitProvisionalLoad, didClearWindowObject,
didCreateScriptContext, didCreateDocumentElement, didCreateNewDocument,
didHandleOnloadEvents, didFinishLoad, etc) interact, how they're ordered, etc.

And try to eliminate some. There's way too many, and it makes this sort of thing
really hard to reason about.