Remove calls to HandleScope default ctor.

Remove calls to HandleScope default ctor.

It's replaced by a version which takes an isolate. The default ctor will be removed soon.

BUG=

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

[marja, 2013-09-02 07:49:20]

dcarney, ptal

[dcarney, 2013-09-02 07:54:20]

lgtm

[haraken, 2013-09-02 08:05:55]

LGTM

https://codereview.chromium.org/23788005/diff/1/Source/bindings/v8/V8Binding.cpp
File Source/bindings/v8/V8Binding.cpp (right):

https://codereview.chromium.org/23788005/diff/1/Source/bindings/v8/V8Binding....
Source/bindings/v8/V8Binding.cpp:593: v8::Isolate*
getIsolateFromScriptExecutionContext(ScriptExecutionContext* context)

The implementation of this method looks hacky. A better way to retrieve an
Isolate would be:

a) Get a V8 context from ScriptExecutionContext. Then, get an Isolate from the
V8 context. (This is the common way we currently retrieve the isolate from
ScriptExecutionContext.)

b) Cache an isolate on ScriptExecutionContext so that we can retrieve the
isolate using context->isolate().

FIXME is fine for now, but please consider implementing a) or b) in a follow-up
CL.

https://codereview.chromium.org/23788005/diff/1/Source/bindings/v8/V8Binding....
Source/bindings/v8/V8Binding.cpp:595: if (context->isDocument()) {

Nit: You might want to add ASSERT(context).

[marja, 2013-09-02 09:05:41]

thanks for review!

https://codereview.chromium.org/23788005/diff/1/Source/bindings/v8/V8Binding.cpp
File Source/bindings/v8/V8Binding.cpp (right):

https://codereview.chromium.org/23788005/diff/1/Source/bindings/v8/V8Binding....
Source/bindings/v8/V8Binding.cpp:593: v8::Isolate*
getIsolateFromScriptExecutionContext(ScriptExecutionContext* context)
On 2013/09/02 08:05:56, haraken wrote:
> 
> The implementation of this method looks hacky. A better way to retrieve an
> Isolate would be:
> 
> a) Get a V8 context from ScriptExecutionContext. Then, get an Isolate from the
> V8 context. (This is the common way we currently retrieve the isolate from
> ScriptExecutionContext.)
> 
> b) Cache an isolate on ScriptExecutionContext so that we can retrieve the
> isolate using context->isolate().
> 
> FIXME is fine for now, but please consider implementing a) or b) in a
follow-up
> CL.

Added a FIXME to do b).

https://codereview.chromium.org/23788005/diff/1/Source/bindings/v8/V8Binding....
Source/bindings/v8/V8Binding.cpp:595: if (context->isDocument()) {
On 2013/09/02 08:05:56, haraken wrote:
> 
> Nit: You might want to add ASSERT(context).

Done.

[commit-bot: I haz the power, 2013-09-02 09:07:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/23788005/8001

[marja, 2013-09-02 09:18:02]

Offline discussion: removed the FIXME, this is the right way to do it :)

[commit-bot: I haz the power, 2013-09-02 09:18:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/23788005/14001

[commit-bot: I haz the power, 2013-09-02 09:29:37]

Retried try job too often on blink_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_pres...

[marja, 2013-09-02 09:43:10]

Oops, more OWNERS needed.

pfeldman@, could you review the following files
    Source/web/InspectorFrontendClientImpl.cpp
    Source/web/WebBindings.cpp
    Source/web/WebDevToolsFrontendImpl.cpp
    Source/web/WebTestingSupport.cpp
    Source/web/tests/CustomEventTest.cpp
    Source/web/tests/WebFrameTest.cpp

[pfeldman, 2013-09-03 09:40:20]

Source/web lgtm

https://codereview.chromium.org/23788005/diff/14001/Source/web/tests/CustomEv...
File Source/web/tests/CustomEventTest.cpp (right):

https://codereview.chromium.org/23788005/diff/14001/Source/web/tests/CustomEv...
Source/web/tests/CustomEventTest.cpp:109: v8::HandleScope
handleScope(frame->frame()->script()->isolate());
Looks like a pattern. ScriptController::GetIsolate(Frame *frame) could help.

[marja, 2013-09-03 11:09:29]

thanks for review

https://codereview.chromium.org/23788005/diff/14001/Source/web/tests/CustomEv...
File Source/web/tests/CustomEventTest.cpp (right):

https://codereview.chromium.org/23788005/diff/14001/Source/web/tests/CustomEv...
Source/web/tests/CustomEventTest.cpp:109: v8::HandleScope
handleScope(frame->frame()->script()->isolate());
On 2013/09/03 09:40:21, pfeldman wrote:
> Looks like a pattern. ScriptController::GetIsolate(Frame *frame) could help.

Added getIsolateFromFrame to V8Bindings.h (matches
getIsolateFromScriptExecutionContext which was already added by this CL).

[commit-bot: I haz the power, 2013-09-03 11:10:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/23788005/26001

[commit-bot: I haz the power, 2013-09-03 13:35:19]

Retried try job too often on win_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_...

[marja, 2013-09-04 12:14:44]

I ran the win_blink_rel again and now it passed, even if it failed consistently
before (go figure), so cqing this again...

[commit-bot: I haz the power, 2013-09-04 12:15:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/23788005/26001

[commit-bot: I haz the power, 2013-09-04 15:27:43]

Change committed as 157217