Refactors parts of gin:

Refactors parts of gin:
. Runner no longer extends ContextHolder. It will still have a
  ContextHolder, but not own it. This enables a couple of
  things:
  . Runner no longer need own a v8::Context.
  . Runner can be lazily created after the ContextHolder.
. Runner becomes a (mostly) pure virtual interface. This enables an
  implementation to execute through blink rather than v8 directly.
. What was Runner is now DefaultRunner (and
  DefaultRunnerDelegate). I'm not a fan of these names, if you have
  better ideas let me know. Maybe DirectRunner?

BUG=none
TEST=none
R=abarth@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=253732

[sky, 2014-02-25 22:39:33]

I'm happy to split this up if you prefer.

https://codereview.chromium.org/179803007/diff/20001/gin/default_runner.h
File gin/default_runner.h (right):

https://codereview.chromium.org/179803007/diff/20001/gin/default_runner.h#new...
gin/default_runner.h:5: #ifndef GIN_DEFAULT_RUNNER_H_
This is really a move, but as so much changed git cl isn't picking it up as
such. *SIGH*

[abarth-chromium, 2014-02-26 05:57:00]

Maybe StandaloneRunner rather than DefaultRunner?

[abarth-chromium, 2014-02-26 06:08:44]

This CL generally looks good, but I'd prefer if we split the ModuleLoader part
out into a separate CL.  That seems like something slightly unrelated.

We should think of some better names.  "Default" doesn't make much sense as a
name.  Default for what?  For Runner, "Standalone" might make sense as a name. 
Another possibility is to call it a ShellRunner because its used for
gin_shell-like functionality.

https://codereview.chromium.org/179803007/diff/60001/gin/modules/module_runne...
File gin/modules/module_runner_delegate.cc (right):

https://codereview.chromium.org/179803007/diff/60001/gin/modules/module_runne...
gin/modules/module_runner_delegate.cc:16: class DefaultModuleLoader : public
ModuleLoader {
Is there a better term to use here than "default"?

What other subclasses of ModuleLoader could there be?  If there are other
sources of modules, I'd have expected them to plug into ModuleRegistry as some
sort of ModuleProvider (i.e., hook in on the backend of the loading process
instead of the frontend).

https://codereview.chromium.org/179803007/diff/60001/gin/modules/module_runne...
File gin/modules/module_runner_delegate.h (right):

https://codereview.chromium.org/179803007/diff/60001/gin/modules/module_runne...
gin/modules/module_runner_delegate.h:36: // From RunnerDelegate:
This comment is out of date

[sky, 2014-02-26 15:02:00]

I removed the ModuleLoader changes from this patch. Take another look?

[sky, 2014-02-26 15:02:54]

Also, I renamed DefaultRunner* to ShellRunner*. I liked both of your suggestions
and didn't feel strongly one way or the other.

[jochen (gone - plz use gerrit), 2014-02-26 15:05:28]

https://codereview.chromium.org/179803007/diff/100001/gin/runner.h
File gin/runner.h (right):

https://codereview.chromium.org/179803007/diff/100001/gin/runner.h#newcode39
gin/runner.h:39: base::WeakPtr<Runner> GetWeakPtr() {
Isn't the intention that blink will implement this api? Then we can't have base/
dependencies

[sky, 2014-02-26 15:08:55]

https://codereview.chromium.org/179803007/diff/100001/gin/runner.h
File gin/runner.h (right):

https://codereview.chromium.org/179803007/diff/100001/gin/runner.h#newcode39
gin/runner.h:39: base::WeakPtr<Runner> GetWeakPtr() {
On 2014/02/26 15:05:28, jochen wrote:
> Isn't the intention that blink will implement this api? Then we can't have
base/
> dependencies

It'll be implemented in content (and redirect to blink). So the base dependency
is fine.

[jochen (gone - plz use gerrit), 2014-02-26 15:31:51]

I see.

Currently, it's not possible to call v8 functions directly from outside of blink
(we ran into this problem while moving some of the CppBoundClasses to gin).

The solution is probably to move the management of recursion scopes from blink
to gin.

[jochen (gone - plz use gerrit), 2014-02-26 15:32:06]

On 2014/02/26 15:31:51, jochen wrote:
> I see.
> 
> Currently, it's not possible to call v8 functions directly from outside of
blink
> (we ran into this problem while moving some of the CppBoundClasses to gin).
> 
> The solution is probably to move the management of recursion scopes from blink
> to gin.

See https://codereview.chromium.org/177553002/

[sky, 2014-02-26 15:33:36]

I was going to call into the WebFrame methods
(executeScript/callFunctionEvenIfScriptDisabled). This seems to work,
of course I haven't tested it exhaustively.

  -Scott

On Wed, Feb 26, 2014 at 7:31 AM,  <jochen@chromium.org> wrote:
> I see.
>
> Currently, it's not possible to call v8 functions directly from outside of
> blink
> (we ran into this problem while moving some of the CppBoundClasses to gin).
>
> The solution is probably to move the management of recursion scopes from
> blink
> to gin.
>
> https://codereview.chromium.org/179803007/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jochen (gone - plz use gerrit), 2014-02-26 15:36:28]

On 2014/02/26 15:33:36, sky wrote:
> I was going to call into the WebFrame methods
> (executeScript/callFunctionEvenIfScriptDisabled). This seems to work,
> of course I haven't tested it exhaustively.

hum, interesting. That might work for the other cases we have as well.

[sky, 2014-02-26 15:41:32]

Adam had suggested doing this route.

  -Scott

On Wed, Feb 26, 2014 at 7:36 AM,  <jochen@chromium.org> wrote:
> On 2014/02/26 15:33:36, sky wrote:
>>
>> I was going to call into the WebFrame methods
>> (executeScript/callFunctionEvenIfScriptDisabled). This seems to work,
>> of course I haven't tested it exhaustively.
>
>
> hum, interesting. That might work for the other cases we have as well.
>
> https://codereview.chromium.org/179803007/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[abarth-chromium, 2014-02-26 19:07:38]

lgtm

[sky, 2014-02-26 21:36:09]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2014-02-26 21:39:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sky@chromium.org/179803007/120001

[commit-bot: I haz the power, 2014-02-27 06:08:26]

Change committed as 253732