DevTools: remove DOM and Bindings dependencies from inspector/v8.

DevTools: remove DOM and Bindings dependencies from inspector/v8.

BUG=580337
Committed: https://crrev.com/c538f43951cc347d629abd5f17291cec9d6bbcf8
Cr-Commit-Position: refs/heads/master@{#372578}

[pfeldman, 2016-01-29 23:08:07]

pfeldman@chromium.org changed reviewers:
+ dgozman@chromium.org

[dgozman, 2016-01-29 23:29:37]

We'll have to do something with HTMLAllCollection and Node in
InjectedScriptSource.

V8StringUtils.{h,cpp} are missing.

https://codereview.chromium.org/1650283002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/inspector/v8/V8DebuggerImpl.cpp (left):

https://codereview.chromium.org/1650283002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/v8/V8DebuggerImpl.cpp:768: return
m_isolate->ThrowException(v8::String::NewFromUtf8(m_isolate, "Debugging is not
enabled.", v8::NewStringType::kNormal).ToLocalChecked());
Why removed ToLocalChecked?

[pfeldman, 2016-01-30 02:44:01]

PTAL

https://codereview.chromium.org/1650283002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/inspector/v8/V8DebuggerImpl.cpp (left):

https://codereview.chromium.org/1650283002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/v8/V8DebuggerImpl.cpp:768: return
m_isolate->ThrowException(v8::String::NewFromUtf8(m_isolate, "Debugging is not
enabled.", v8::NewStringType::kNormal).ToLocalChecked());
On 2016/01/29 23:29:37, dgozman wrote:
> Why removed ToLocalChecked?

Done.

[pfeldman, 2016-01-30 06:59:45]

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

[commit-bot: I haz the power, 2016-01-30 06:59:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1650283002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1650283002/40001

[commit-bot: I haz the power, 2016-01-30 08:25:29]

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

[commit-bot: I haz the power, 2016-01-30 08:25:30]

Dry run: This issue passed the CQ dry run.

[pfeldman, 2016-01-30 15:28:19]

pfeldman@chromium.org changed reviewers:
+ jochen@chromium.org

[pfeldman, 2016-01-30 15:28:50]

Description was changed from

==========
DevTools: remove DOM and Bindings dependencies from inspector/v8.

BUG=580337
==========

to

==========
DevTools: remove DOM and Bindings dependencies from inspector/v8.

BUG=580337
==========

[pfeldman, 2016-01-30 15:28:50]

pfeldman@chromium.org changed reviewers:
+ yurys@chromium.org

[dgozman, 2016-01-30 16:36:39]

lgtm

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/v8/InjectedScriptSource.js
(right):

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/v8/InjectedScriptSource.js:1697: return
!!node && InjectedScriptHost.subtype(node) === "node" && (node.nodeType ===
Node.ELEMENT_NODE || node.nodeType === Node.DOCUMENT_NODE || node.nodeType ===
Node.DOCUMENT_FRAGMENT_NODE);
I was talking about places like this.

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/v8/V8InjectedScriptHost.cpp
(right):

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/v8/V8InjectedScriptHost.cpp:59: if
(!info[0]->IsInt32() && !isolate->IsExecutionTerminating()) {
You have to return from the method even if execution is terminating.

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/v8/V8StringUtil.cpp (right):

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/v8/V8StringUtil.cpp:24: String result =
String::createUninitialized(value->Length(), buffer);
This signature made me dig up who is allocating the memory.

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/elements/elementsTreeOutline.css
(right):

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/elementsTreeOutline.css:32:
opacity: 0.6;
That's another patch.

[pfeldman, 2016-01-31 16:04:13]

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/v8/InjectedScriptSource.js
(right):

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/v8/InjectedScriptSource.js:1697: return
!!node && InjectedScriptHost.subtype(node) === "node" && (node.nodeType ===
Node.ELEMENT_NODE || node.nodeType === Node.DOCUMENT_NODE || node.nodeType ===
Node.DOCUMENT_FRAGMENT_NODE);
On 2016/01/30 16:36:39, dgozman wrote:
> I was talking about places like this.

Added to the backlog.

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/v8/V8InjectedScriptHost.cpp
(right):

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/v8/V8InjectedScriptHost.cpp:59: if
(!info[0]->IsInt32() && !isolate->IsExecutionTerminating()) {
On 2016/01/30 16:36:39, dgozman wrote:
> You have to return from the method even if execution is terminating.
I'm not changing the behavior and v8SetReturnValue would set return value.

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/v8/V8StringUtil.cpp (right):

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/v8/V8StringUtil.cpp:24: String result =
String::createUninitialized(value->Length(), buffer);
On 2016/01/30 16:36:39, dgozman wrote:
> This signature made me dig up who is allocating the memory.

That's the way we do it in bindings and other v8 call sites in chrome.

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/elements/elementsTreeOutline.css
(right):

https://codereview.chromium.org/1650283002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/elementsTreeOutline.css:32:
opacity: 0.6;
On 2016/01/30 16:36:39, dgozman wrote:
> That's another patch.

Ouch.

[pfeldman, 2016-01-31 16:10:28]

The CQ bit was checked by pfeldman@chromium.org

[pfeldman, 2016-01-31 16:10:28]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/1650283002/#ps60001
(title: "for landing")

[commit-bot: I haz the power, 2016-01-31 16:10:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1650283002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1650283002/60001

[commit-bot: I haz the power, 2016-01-31 17:20:51]

Description was changed from

==========
DevTools: remove DOM and Bindings dependencies from inspector/v8.

BUG=580337
==========

to

==========
DevTools: remove DOM and Bindings dependencies from inspector/v8.

BUG=580337
==========

[commit-bot: I haz the power, 2016-01-31 17:20:51]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-01-31 17:22:05]

Description was changed from

==========
DevTools: remove DOM and Bindings dependencies from inspector/v8.

BUG=580337
==========

to

==========
DevTools: remove DOM and Bindings dependencies from inspector/v8.

BUG=580337

Committed: https://crrev.com/c538f43951cc347d629abd5f17291cec9d6bbcf8
Cr-Commit-Position: refs/heads/master@{#372578}
==========

[commit-bot: I haz the power, 2016-01-31 17:22:06]

Patchset 4 (id:??) landed as
https://crrev.com/c538f43951cc347d629abd5f17291cec9d6bbcf8
Cr-Commit-Position: refs/heads/master@{#372578}

[dgozman, 2016-02-01 17:06:24]

https://codereview.chromium.org/1650283002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/v8/V8InjectedScriptHost.cpp
(right):

https://codereview.chromium.org/1650283002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/v8/V8InjectedScriptHost.cpp:59: if
(!info[0]->IsInt32() && !isolate->IsExecutionTerminating()) {
I still think that during execution termination As<v8::Int32> may return null
which will be dereferenced.