Move Extension ScriptContext creation into ScriptContextSet.

Move Extension ScriptContext creation into ScriptContextSet.

Previously extensions::Dispatcher was responsible for creating ScriptContexts
then Adding them to its ScriptContextSet. Now, ScriptContextSet creates them
itself. This prepares the Extension system for later changing to lazily or
asynchronously creating ScriptContexts.

BUG=466373
R=rockot@chromium.org

Committed: https://crrev.com/745b66237eca67b22225941eb475ba8a995e1287
Cr-Commit-Position: refs/heads/master@{#320971}

[not at google - send to devlin, 2015-03-12 16:59:57]

https://codereview.chromium.org/995283004/diff/20001/extensions/renderer/scri...
File extensions/renderer/script_context_set.h (right):

https://codereview.chromium.org/995283004/diff/20001/extensions/renderer/scri...
extensions/renderer/script_context_set.h:48: // Must outlive this. TODO(kalman):
Combine this and |extensions|.
FYI by "combine this" I mean that the renderer side of storing a set of
extensions would be better as a new ExtensionSet-style class with Is/SetActive()
methods on it, rather than this awkward parallel set.

https://codereview.chromium.org/995283004/diff/20001/extensions/renderer/scri...
extensions/renderer/script_context_set.h:59: ScriptContext*
Register(blink::WebLocalFrame* frame,
I'm not particularly proud of this function being called "Register" - though
imagine a world in which instead of returning a ScriptContext* it takes a
base::Callback<void(ScriptContext*)> and returns void. I.e. lazily creates these
ScriptContexts*. That should, hopefully be coming up next.

(and I avoided Create because that almost always implies that ownership is
passed to caller)

[Ken Rockot(use gerrit already), 2015-03-12 22:45:16]

lgtm

https://codereview.chromium.org/995283004/diff/20001/extensions/renderer/scri...
File extensions/renderer/script_context_set.cc (right):

https://codereview.chromium.org/995283004/diff/20001/extensions/renderer/scri...
extensions/renderer/script_context_set.cc:143: // TODO(kalman): Do something
here?
Unless I'm reading this wrong, is this no longer recording UMA that was being
recorded before in this case?

https://codereview.chromium.org/995283004/diff/20001/extensions/renderer/scri...
File extensions/renderer/script_context_set.h (right):

https://codereview.chromium.org/995283004/diff/20001/extensions/renderer/scri...
extensions/renderer/script_context_set.h:59: ScriptContext*
Register(blink::WebLocalFrame* frame,
On 2015/03/12 16:59:57, kalman wrote:
> I'm not particularly proud of this function being called "Register" - though
> imagine a world in which instead of returning a ScriptContext* it takes a
> base::Callback<void(ScriptContext*)> and returns void. I.e. lazily creates
these
> ScriptContexts*. That should, hopefully be coming up next.
> 
> (and I avoided Create because that almost always implies that ownership is
> passed to caller)

Seems fine to me. Doubly so with the future callback.

[not at google - send to devlin, 2015-03-12 22:48:34]

Thanks

https://codereview.chromium.org/995283004/diff/20001/extensions/renderer/scri...
File extensions/renderer/script_context_set.cc (right):

https://codereview.chromium.org/995283004/diff/20001/extensions/renderer/scri...
extensions/renderer/script_context_set.cc:143: // TODO(kalman): Do something
here?
On 2015/03/12 22:45:16, Ken Rockot wrote:
> Unless I'm reading this wrong, is this no longer recording UMA that was being
> recorded before in this case?

Yes, I added that UMA maybe 4 years ago? It was never actually added to
actions.xml so was never recorded - and it's rather boring anyway.

https://codereview.chromium.org/995283004/diff/20001/extensions/renderer/scri...
File extensions/renderer/script_context_set.h (right):

https://codereview.chromium.org/995283004/diff/20001/extensions/renderer/scri...
extensions/renderer/script_context_set.h:59: ScriptContext*
Register(blink::WebLocalFrame* frame,
On 2015/03/12 22:45:16, Ken Rockot wrote:
> On 2015/03/12 16:59:57, kalman wrote:
> > I'm not particularly proud of this function being called "Register" - though
> > imagine a world in which instead of returning a ScriptContext* it takes a
> > base::Callback<void(ScriptContext*)> and returns void. I.e. lazily creates
> these
> > ScriptContexts*. That should, hopefully be coming up next.
> > 
> > (and I avoided Create because that almost always implies that ownership is
> > passed to caller)
> 
> Seems fine to me. Doubly so with the future callback.

Alrightee.

[not at google - send to devlin, 2015-03-12 22:48:47]

The CQ bit was checked by kalman@chromium.org

[commit-bot: I haz the power, 2015-03-12 22:48:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/995283004/20001

[commit-bot: I haz the power, 2015-03-12 22:52:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-12 22:52:09]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[not at google - send to devlin, 2015-03-13 22:16:19]

The CQ bit was checked by kalman@chromium.org

[not at google - send to devlin, 2015-03-13 22:16:19]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org
Link to the patchset: https://codereview.chromium.org/995283004/#ps40001 (title:
"rebase")

[commit-bot: I haz the power, 2015-03-13 22:16:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/995283004/40001

[commit-bot: I haz the power, 2015-03-13 23:07:28]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-13 23:08:08]

Patchset 3 (id:??) landed as
https://crrev.com/75605c7fca774b95984ba67f70ba86bb5750c0f6
Cr-Commit-Position: refs/heads/master@{#320602}

[not at google - send to devlin, 2015-03-14 00:50:56]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1010473002/ by kalman@chromium.org.

The reason for reverting is: Broke ScriptContextTest.GinRunnerLifetime..

[not at google - send to devlin, 2015-03-16 18:17:23]

The CQ bit was checked by kalman@chromium.org

[not at google - send to devlin, 2015-03-16 18:17:24]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org
Link to the patchset: https://codereview.chromium.org/995283004/#ps60001 (title:
"fix gin memory leak")

[commit-bot: I haz the power, 2015-03-16 18:18:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/995283004/60001

[commit-bot: I haz the power, 2015-03-16 19:16:36]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-03-16 19:17:36]

Patchset 4 (id:??) landed as
https://crrev.com/52f4caf6d01302ebf42136b42fa77d1c67d93ae9
Cr-Commit-Position: refs/heads/master@{#320770}

[not at google - send to devlin, 2015-03-16 21:18:19]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1013823003/ by kalman@chromium.org.

The reason for reverting is: Memory failure in
ScriptContextTest.GinRunnerLifetime..

[not at google - send to devlin, 2015-03-17 20:01:41]

The CQ bit was checked by kalman@chromium.org

[not at google - send to devlin, 2015-03-17 20:01:42]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org
Link to the patchset: https://codereview.chromium.org/995283004/#ps100001
(title: "fix memory for real")

[commit-bot: I haz the power, 2015-03-17 20:01:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/995283004/100001

[commit-bot: I haz the power, 2015-03-17 21:07:27]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-03-17 21:08:43]

Patchset 6 (id:??) landed as
https://crrev.com/745b66237eca67b22225941eb475ba8a995e1287
Cr-Commit-Position: refs/heads/master@{#320971}