[DevTools] Move CommandLineAPI querySelector and querySelectorAll to native

[DevTools] Move CommandLineAPI querySelector and querySelectorAll to native

BUG=595206
R=dgozman@chromium.org

Committed: https://crrev.com/90178e211ae004f70fd4152343055164fce4ce43
Cr-Commit-Position: refs/heads/master@{#394977}

[kozy, 2016-05-18 01:30:39]

please take a look!

[dgozman, 2016-05-18 01:51:26]

https://codereview.chromium.org/1983423002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp (right):

https://codereview.chromium.org/1983423002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp:282:
info.GetReturnValue().Set(toV8(static_cast<Node*>(element), info.Holder(),
info.GetIsolate()));
I think static_cast is redundant.

https://codereview.chromium.org/1983423002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp:295:
StaticElementList* elementList =
toContainerNode(node)->querySelectorAll(AtomicString(selector),
IGNORE_EXCEPTION);
// toV8(elementList) doesn't work here, since we need a proper Array instance,
not NodeList.

https://codereview.chromium.org/1983423002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/v8_inspector/public/V8DebuggerClient.h
(right):

https://codereview.chromium.org/1983423002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/v8_inspector/public/V8DebuggerClient.h:38:
virtual void willInstallCommandLineAPI(v8::Local<v8::Context>,
v8::Local<v8::Object>) = 0;
installAdditionalCommandLineAPI

[kozy, 2016-05-18 02:18:56]

All done, xpath selector added, ptal!

https://codereview.chromium.org/1983423002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp (right):

https://codereview.chromium.org/1983423002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp:282:
info.GetReturnValue().Set(toV8(static_cast<Node*>(element), info.Holder(),
info.GetIsolate()));
On 2016/05/18 01:51:26, dgozman wrote:
> I think static_cast is redundant.

Removed.

https://codereview.chromium.org/1983423002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp:295:
StaticElementList* elementList =
toContainerNode(node)->querySelectorAll(AtomicString(selector),
IGNORE_EXCEPTION);
On 2016/05/18 01:51:26, dgozman wrote:
> // toV8(elementList) doesn't work here, since we need a proper Array instance,
> not NodeList.

Done.

https://codereview.chromium.org/1983423002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/v8_inspector/public/V8DebuggerClient.h
(right):

https://codereview.chromium.org/1983423002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/v8_inspector/public/V8DebuggerClient.h:38:
virtual void willInstallCommandLineAPI(v8::Local<v8::Context>,
v8::Local<v8::Object>) = 0;
On 2016/05/18 01:51:26, dgozman wrote:
> installAdditionalCommandLineAPI

Done.

[dgozman, 2016-05-18 03:30:18]

I think xpath implementation has issues. Do we have tests for this?

https://codereview.chromium.org/1983423002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp (right):

https://codereview.chromium.org/1983423002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp:322: node =
node->ownerDocument();
I think you lose node here, while it's important in evaluate call.

[kozy, 2016-05-18 22:46:40]

All done, added a test, please take a look.

https://codereview.chromium.org/1983423002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp (right):

https://codereview.chromium.org/1983423002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp:322: node =
node->ownerDocument();
On 2016/05/18 03:30:18, dgozman wrote:
> I think you lose node here, while it's important in evaluate call.

Done.

[dgozman, 2016-05-18 23:15:18]

lgtm

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

https://codereview.chromium.org/1983423002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp:334:
ExceptionState exceptionState(ExceptionState::ExecutionContext, "$x",
"CommandLineAPI", info.Holder(), info.GetIsolate());
Do we surface this exception in console? Previously we did.

[kozy, 2016-05-19 00:45:50]

Patchset #4 (id:60001) has been deleted

[kozy, 2016-05-19 00:45:59]

All done.

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

https://codereview.chromium.org/1983423002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp:334:
ExceptionState exceptionState(ExceptionState::ExecutionContext, "$x",
"CommandLineAPI", info.Holder(), info.GetIsolate());
On 2016/05/18 23:15:18, dgozman wrote:
> Do we surface this exception in console? Previously we did.

Done.

[kozy, 2016-05-19 00:46:22]

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

[kozy, 2016-05-19 00:46:23]

Pavel, please take a look. I need owner lgtm for V8WindowCustom.

[pfeldman, 2016-05-19 23:10:13]

lgtm

[kozy, 2016-05-19 23:30:18]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2016-05-19 23:30:18]

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/1983423002/#ps120001
(title: " ")

[commit-bot: I haz the power, 2016-05-19 23:32:23]

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

[commit-bot: I haz the power, 2016-05-20 00:49:05]

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

[commit-bot: I haz the power, 2016-05-20 00:49:06]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[kozy, 2016-05-20 01:07:13]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2016-05-20 01:07:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/1983423002/#ps140001
(title: "rebased")

[commit-bot: I haz the power, 2016-05-20 01:11:54]

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

[commit-bot: I haz the power, 2016-05-20 03:20:43]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2016-05-20 03:22:32]

Description was changed from

==========
[DevTools] Move CommandLineAPI querySelector and querySelectorAll to native

BUG=595206
R=dgozman@chromium.org
==========

to

==========
[DevTools] Move CommandLineAPI querySelector and querySelectorAll to native

BUG=595206
R=dgozman@chromium.org

Committed: https://crrev.com/90178e211ae004f70fd4152343055164fce4ce43
Cr-Commit-Position: refs/heads/master@{#394977}
==========

[commit-bot: I haz the power, 2016-05-20 03:22:33]

Patchset 7 (id:??) landed as
https://crrev.com/90178e211ae004f70fd4152343055164fce4ce43
Cr-Commit-Position: refs/heads/master@{#394977}