[DevTools] Removed ScriptState from createScriptCallStackForConsole

[DevTools] Removed ScriptState from createScriptCallTackForConsole

In method createScriptCallTackFromConsole we can get current execution context in function body without passing it in arguments.

Removed canGenerateCallStack argument from InspectorConsoleMessage cstor which uses the transmitted stack.

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

[kozyatinskiy1, 2014-08-11 15:28:21]

@vsevik, please take a look.

[kozyatinskiy1, 2014-08-11 15:34:21]

@pfeldman, please take a look. I need owner review for Source/bindings/* files.

[vsevik, 2014-08-11 15:52:04]

https://chromiumcodereview.appspot.com/458253002/diff/1/Source/bindings/core/...
File Source/bindings/core/v8/ScriptCallStackFactory.cpp (right):

https://chromiumcodereview.appspot.com/458253002/diff/1/Source/bindings/core/...
Source/bindings/core/v8/ScriptCallStackFactory.cpp:119: if
(InspectorInstrumentation::consoleAgentEnabled(currentExecutionContext(isolate)))
These lines might be needed here, please check with yurys@
if (!isolate->InContext())
    return nullptr;
v8::HandleScope scope(isolate);

https://chromiumcodereview.appspot.com/458253002/diff/1/Source/core/inspector...
File Source/core/inspector/InspectorConsoleAgent.cpp (right):

https://chromiumcodereview.appspot.com/458253002/diff/1/Source/core/inspector...
Source/core/inspector/InspectorConsoleAgent.cpp:172:
addConsoleMessage(adoptPtr(new InspectorConsoleMessage(false,
consoleMessage->source(), LogMessageType, consoleMessage->level(),
consoleMessage->message(), consoleMessage->callStack(),
consoleMessage->requestIdentifier())));
Let's remove this parameter.

https://chromiumcodereview.appspot.com/458253002/diff/1/Source/core/inspector...
File Source/core/inspector/InspectorProfilerAgent.cpp (right):

https://chromiumcodereview.appspot.com/458253002/diff/1/Source/core/inspector...
Source/core/inspector/InspectorProfilerAgent.cpp:69:
RefPtrWillBeRawPtr<ScriptCallStack>
callStack(createScriptCallStackForConsole(1));
You could simply call createScriptCallStack(1) here

[kozyatinskiy1, 2014-08-11 16:05:35]

https://codereview.chromium.org/458253002/diff/1/Source/bindings/core/v8/Scri...
File Source/bindings/core/v8/ScriptCallStackFactory.cpp (right):

https://codereview.chromium.org/458253002/diff/1/Source/bindings/core/v8/Scri...
Source/bindings/core/v8/ScriptCallStackFactory.cpp:119: if
(InspectorInstrumentation::consoleAgentEnabled(currentExecutionContext(isolate)))
On 2014/08/11 15:52:03, vsevik wrote:
> These lines might be needed here, please check with yurys@
> if (!isolate->InContext())
>     return nullptr;
> v8::HandleScope scope(isolate);

First check added. @yurus, could you please answer: do we really need a scope in
this case?

https://codereview.chromium.org/458253002/diff/1/Source/core/inspector/Inspec...
File Source/core/inspector/InspectorConsoleAgent.cpp (right):

https://codereview.chromium.org/458253002/diff/1/Source/core/inspector/Inspec...
Source/core/inspector/InspectorConsoleAgent.cpp:172:
addConsoleMessage(adoptPtr(new InspectorConsoleMessage(false,
consoleMessage->source(), LogMessageType, consoleMessage->level(),
consoleMessage->message(), consoleMessage->callStack(),
consoleMessage->requestIdentifier())));
On 2014/08/11 15:52:03, vsevik wrote:
> Let's remove this parameter.

Done.

https://codereview.chromium.org/458253002/diff/1/Source/core/inspector/Inspec...
File Source/core/inspector/InspectorProfilerAgent.cpp (right):

https://codereview.chromium.org/458253002/diff/1/Source/core/inspector/Inspec...
Source/core/inspector/InspectorProfilerAgent.cpp:69:
RefPtrWillBeRawPtr<ScriptCallStack>
callStack(createScriptCallStackForConsole(1));
On 2014/08/11 15:52:03, vsevik wrote:
> You could simply call createScriptCallStack(1) here

Done.

[aandrey, 2014-08-11 17:38:03]

https://codereview.chromium.org/458253002/diff/1/Source/bindings/core/v8/Scri...
File Source/bindings/core/v8/ScriptCallStackFactory.cpp (right):

https://codereview.chromium.org/458253002/diff/1/Source/bindings/core/v8/Scri...
Source/bindings/core/v8/ScriptCallStackFactory.cpp:119: if
(InspectorInstrumentation::consoleAgentEnabled(currentExecutionContext(isolate)))
On 2014/08/11 16:05:34, kozyatinskiy wrote:
> On 2014/08/11 15:52:03, vsevik wrote:
> > These lines might be needed here, please check with yurys@
> > if (!isolate->InContext())
> >     return nullptr;
> > v8::HandleScope scope(isolate);
> 
> First check added. @yurus, could you please answer: do we really need a scope
in
> this case?

1) Calling currentExecutionContext() when not in context should be fine since
https://codereview.chromium.org/16943006

2) You don't use v8::Handle<>'s, so handle scope not needed.

[kozyatinskiy1, 2014-08-11 17:50:42]

https://codereview.chromium.org/458253002/diff/1/Source/bindings/core/v8/Scri...
File Source/bindings/core/v8/ScriptCallStackFactory.cpp (right):

https://codereview.chromium.org/458253002/diff/1/Source/bindings/core/v8/Scri...
Source/bindings/core/v8/ScriptCallStackFactory.cpp:119: if
(InspectorInstrumentation::consoleAgentEnabled(currentExecutionContext(isolate)))
On 2014/08/11 17:38:03, aandrey wrote:
> On 2014/08/11 16:05:34, kozyatinskiy wrote:
> > On 2014/08/11 15:52:03, vsevik wrote:
> > > These lines might be needed here, please check with yurys@
> > > if (!isolate->InContext())
> > >     return nullptr;
> > > v8::HandleScope scope(isolate);
> > 
> > First check added. @yurus, could you please answer: do we really need a
scope
> in
> > this case?
> 
> 1) Calling currentExecutionContext() when not in context should be fine since
> https://codereview.chromium.org/16943006
> 
> 2) You don't use v8::Handle<>'s, so handle scope not needed.

Thank you. Furthermore, this test may be useful here for a quick exit from the
function.

[yurys, 2014-08-12 08:12:50]

lgtm

[commit-bot: I haz the power, 2014-08-12 08:13:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kozyatinskiy@google.com/458253002/40001

[commit-bot: I haz the power, 2014-08-12 09:23:27]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/22228)

[commit-bot: I haz the power, 2014-08-12 10:15:49]

Change committed as 180057