DevTools: Runtime.compileScript with persistScript = false should not report script

DevTools: Runtime.compileScript with persistScript = false should not report script

The Runtime.complieScript with persistScript = false flag is used to
dynamically report compilation error while user is typing in javascript.

There's no good reason this function reports scripts to front-end.
This leads to a memory leak, and also spoils the "search across all files"
experience (all the script replicas are searchable as well).

BUG=none
R=dgozman, kozyatinskiy

Committed: https://crrev.com/2ff2c6e7664bcd26d82800023a14f754143a6052
Cr-Commit-Position: refs/heads/master@{#410891}

[lushnikov, 2016-08-09 03:08:21]

Description was changed from

==========
DevTools: Runtime.compileScript with persistScript = false should not report
script

The Runtime.complieScript with persistScript = false flag is used to
dynamically report compilation error while user is typing in javascript.

There's no good reason this function reports scripts to front-end.
This leads to a memory leak, and also spoils the "search across all files"
experience (all the script replicas are searchable as well).

BUG=none
R=dgozman
==========

to

==========
DevTools: Runtime.compileScript with persistScript = false should not report
script

The Runtime.complieScript with persistScript = false flag is used to
dynamically report compilation error while user is typing in javascript.

There's no good reason this function reports scripts to front-end.
This leads to a memory leak, and also spoils the "search across all files"
experience (all the script replicas are searchable as well).

BUG=none
R=dgozman, kozyatinskiy
==========

[lushnikov, 2016-08-09 03:08:21]

lushnikov@chromium.org changed reviewers:
+ kozyatinskiy@chromium.org

[lushnikov, 2016-08-09 03:11:38]

please, take a look

[kozy, 2016-08-09 16:25:44]

https://codereview.chromium.org/2219393003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/inspector-protocol/runtime/runtime-compileScript.html
(right):

https://codereview.chromium.org/2219393003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/inspector-protocol/runtime/runtime-compileScript.html:9:
InspectorTest.sendCommand("Debugger.enable", {}, onDebuggerEnabled);
We can use more promises!
InspectorTest.sendCommandPromise("Debugger.enable", {})
  .then(() => InspectorTest.sendCommandPromise("Runtime.enable", {}))
  .then(() =>
InspectorTest.waitForEventPromise("Runtime.executionContextCreated")
  .then((msg) => onExecutionContextCreated(msg))
  .then(() => testCompileScript(...))
...
  .then(() => InspectorTest.completeTest()

Or leave it as is.

https://codereview.chromium.org/2219393003/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp
(right):

https://codereview.chromium.org/2219393003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:1170:
void V8DebuggerAgentImpl::setIgnoreScriptParsedEvents(bool ignore)
Can we ignore this events in V8Debugger because V8Debugger calls to
DebuggerScript.js and creates a V8DebuggerScript and in constructor of this
method calculates hash of script source?

[lushnikov, 2016-08-09 20:16:13]

https://codereview.chromium.org/2219393003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/inspector-protocol/runtime/runtime-compileScript.html
(right):

https://codereview.chromium.org/2219393003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/inspector-protocol/runtime/runtime-compileScript.html:9:
InspectorTest.sendCommand("Debugger.enable", {}, onDebuggerEnabled);
Unfortunately, there's no waitForEventPromise! Without it the whole chain
breaks..

I'll leave it as is.

https://codereview.chromium.org/2219393003/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp
(right):

https://codereview.chromium.org/2219393003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/v8_inspector/V8DebuggerAgentImpl.cpp:1170:
void V8DebuggerAgentImpl::setIgnoreScriptParsedEvents(bool ignore)
On 2016/08/09 16:25:44, kozyatinskiy wrote:
> Can we ignore this events in V8Debugger because V8Debugger calls to
> DebuggerScript.js and creates a V8DebuggerScript and in constructor of this
> method calculates hash of script source?

Done.

[dgozman, 2016-08-09 20:43:09]

https://codereview.chromium.org/2219393003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector-protocol/runtime/runtime-compileScript.html
(right):

https://codereview.chromium.org/2219393003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector-protocol/runtime/runtime-compileScript.html:41:
hasReportedScript = false;
Unused.

https://codereview.chromium.org/2219393003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/V8Debugger.h (right):

https://codereview.chromium.org/2219393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8Debugger.h:117: bool
m_ignoreScriptParsedEvents;
Let's do the counter to accommodate for nested loop.

https://codereview.chromium.org/2219393003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/V8RuntimeAgentImpl.cpp
(right):

https://codereview.chromium.org/2219393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8RuntimeAgentImpl.cpp:509:
debuggerAgent->setIgnoreScriptParsedEvents(true);
Remote the indirection, call into V8Debugger here.

[lushnikov, 2016-08-09 22:24:29]

https://codereview.chromium.org/2219393003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector-protocol/runtime/runtime-compileScript.html
(right):

https://codereview.chromium.org/2219393003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector-protocol/runtime/runtime-compileScript.html:41:
hasReportedScript = false;
On 2016/08/09 20:43:08, dgozman wrote:
> Unused.

Done.

https://codereview.chromium.org/2219393003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/V8Debugger.h (right):

https://codereview.chromium.org/2219393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8Debugger.h:117: bool
m_ignoreScriptParsedEvents;
On 2016/08/09 20:43:08, dgozman wrote:
> Let's do the counter to accommodate for nested loop.

Done.

https://codereview.chromium.org/2219393003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/V8RuntimeAgentImpl.cpp
(right):

https://codereview.chromium.org/2219393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/V8RuntimeAgentImpl.cpp:509:
debuggerAgent->setIgnoreScriptParsedEvents(true);
On 2016/08/09 20:43:08, dgozman wrote:
> Remote the indirection, call into V8Debugger here.

Done.

[lushnikov, 2016-08-09 22:31:53]

The CQ bit was checked by lushnikov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-09 22:32:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dgozman, 2016-08-09 23:01:34]

Alexey, please coordinate the isInternalScript removal with this.

[kozy, 2016-08-10 00:11:14]

lgtm

[lushnikov, 2016-08-10 00:11:32]

The CQ bit was unchecked by lushnikov@chromium.org

[lushnikov, 2016-08-10 00:11:33]

The CQ bit was checked by lushnikov@chromium.org

[commit-bot: I haz the power, 2016-08-10 00:11:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 00:17:17]

Description was changed from

==========
DevTools: Runtime.compileScript with persistScript = false should not report
script

The Runtime.complieScript with persistScript = false flag is used to
dynamically report compilation error while user is typing in javascript.

There's no good reason this function reports scripts to front-end.
This leads to a memory leak, and also spoils the "search across all files"
experience (all the script replicas are searchable as well).

BUG=none
R=dgozman, kozyatinskiy
==========

to

==========
DevTools: Runtime.compileScript with persistScript = false should not report
script

The Runtime.complieScript with persistScript = false flag is used to
dynamically report compilation error while user is typing in javascript.

There's no good reason this function reports scripts to front-end.
This leads to a memory leak, and also spoils the "search across all files"
experience (all the script replicas are searchable as well).

BUG=none
R=dgozman, kozyatinskiy
==========

[commit-bot: I haz the power, 2016-08-10 00:17:18]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-10 00:21:04]

Description was changed from

==========
DevTools: Runtime.compileScript with persistScript = false should not report
script

The Runtime.complieScript with persistScript = false flag is used to
dynamically report compilation error while user is typing in javascript.

There's no good reason this function reports scripts to front-end.
This leads to a memory leak, and also spoils the "search across all files"
experience (all the script replicas are searchable as well).

BUG=none
R=dgozman, kozyatinskiy
==========

to

==========
DevTools: Runtime.compileScript with persistScript = false should not report
script

The Runtime.complieScript with persistScript = false flag is used to
dynamically report compilation error while user is typing in javascript.

There's no good reason this function reports scripts to front-end.
This leads to a memory leak, and also spoils the "search across all files"
experience (all the script replicas are searchable as well).

BUG=none
R=dgozman, kozyatinskiy

Committed: https://crrev.com/2ff2c6e7664bcd26d82800023a14f754143a6052
Cr-Commit-Position: refs/heads/master@{#410891}
==========

[commit-bot: I haz the power, 2016-08-10 00:21:06]

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