Remove a EscapableHandleScope from executeScriptInIsolatedWorld()

Remove a EscapableHandleScope from executeScriptInIsolatedWorld()

It is wrong to call EscapableHandleScope::Escape() multiple times
in executeScriptInIsolatedWorld(). Since the callers of executeScriptInIsolatedWorld()
already have HandleScopes, we can just simply remove a EscapableHandleScope
from executeScriptInIsolatedWorld().

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185106

[haraken, 2014-11-10 04:08:54]

haraken@chromium.org changed reviewers:
+ dcarney@chromium.org, jochen@chromium.org

[haraken, 2014-11-10 04:08:54]

PTAL

[dcarney, 2014-11-10 08:35:34]

lgtm

it's a little weird that we return a vector of handles - i didn't see any place
in the source that we pass multiple scripts and expect multiple results, but
there are a bunch of places where we try to ensure there is only one result. 
the function here should take a single scripte and use an Escapable handle scope
that escapes one result.  can you just add a TODO?

[haraken, 2014-11-10 08:38:49]

Thanks for review!

> it's a little weird that we return a vector of handles - i didn't see any
place
> in the source that we pass multiple scripts and expect multiple results, but
> there are a bunch of places where we try to ensure there is only one result. 
> the function here should take a single scripte and use an Escapable handle
scope
> that escapes one result.  can you just add a TODO?

This method is exposed to chromium... Added a FIXME.

[haraken, 2014-11-10 08:38:51]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-11-10 08:39:02]

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

[pguyot, 2014-11-10 09:06:06]

On 2014/11/10 08:38:49, haraken wrote:
> Thanks for review!
> 
> > it's a little weird that we return a vector of handles - i didn't see any
> place
> > in the source that we pass multiple scripts and expect multiple results, but
> > there are a bunch of places where we try to ensure there is only one result.

> > the function here should take a single scripte and use an Escapable handle
> scope
> > that escapes one result.  can you just add a TODO?
> 
> This method is exposed to chromium... Added a FIXME.

I'm confused about the FIXME and why this feature should disappear, especially
considering I noticed the issue because we actually pass several scripts and our
tests failed…
Is there an alternate, preferred way to evaluate several scripts within a single
isolated world and get the results back?

See:
https://code.google.com/p/chromiumembedded/issues/detail?id=302

[haraken, 2014-11-10 09:07:17]

On 2014/11/10 09:06:06, pguyot wrote:
> On 2014/11/10 08:38:49, haraken wrote:
> > Thanks for review!
> > 
> > > it's a little weird that we return a vector of handles - i didn't see any
> > place
> > > in the source that we pass multiple scripts and expect multiple results,
but
> > > there are a bunch of places where we try to ensure there is only one
result.
> 
> > > the function here should take a single scripte and use an Escapable handle
> > scope
> > > that escapes one result.  can you just add a TODO?
> > 
> > This method is exposed to chromium... Added a FIXME.
> 
> I'm confused about the FIXME and why this feature should disappear, especially
> considering I noticed the issue because we actually pass several scripts and
our
> tests failed…
> Is there an alternate, preferred way to evaluate several scripts within a
single
> isolated world and get the results back?
> 
> See:
> https://code.google.com/p/chromiumembedded/issues/detail?id=302

Would it be possible to just call executeInIsolatedWorld multiple times?

[pguyot, 2014-11-10 09:22:41]

On 2014/11/10 09:07:17, haraken wrote:
> On 2014/11/10 09:06:06, pguyot wrote:
> > On 2014/11/10 08:38:49, haraken wrote:
> > > Thanks for review!
> > > 
> > > > it's a little weird that we return a vector of handles - i didn't see
any
> > > place
> > > > in the source that we pass multiple scripts and expect multiple results,
> but
> > > > there are a bunch of places where we try to ensure there is only one
> result.
> > 
> > > > the function here should take a single scripte and use an Escapable
handle
> > > scope
> > > > that escapes one result.  can you just add a TODO?
> > > 
> > > This method is exposed to chromium... Added a FIXME.
> > 
> > I'm confused about the FIXME and why this feature should disappear,
especially
> > considering I noticed the issue because we actually pass several scripts and
> our
> > tests failed…
> > Is there an alternate, preferred way to evaluate several scripts within a
> single
> > isolated world and get the results back?
> > 
> > See:
> > https://code.google.com/p/chromiumembedded/issues/detail?id=302
> 
> Would it be possible to just call executeInIsolatedWorld multiple times?

Thanks for the quick reply. It seems to work. I guess it just requires that the
isolated world isn't discarded/modified between calls.

[commit-bot: I haz the power, 2014-11-10 09:33:48]

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

[commit-bot: I haz the power, 2014-11-10 09:33:48]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/3...)

[jochen (gone - plz use gerrit), 2014-11-10 13:37:50]

yes, can we please update the API to only allow executing one script at a time?

[haraken, 2014-11-11 05:39:27]

On 2014/11/10 13:37:50, jochen (OOO) wrote:
> yes, can we please update the API to only allow executing one script at a
time?

sure, will do in a follow-up.

[haraken, 2014-11-11 05:39:42]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-11-11 05:40:20]

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

[commit-bot: I haz the power, 2014-11-11 06:42:12]

Committed patchset #3 (id:40001) as 185106