[DevTools] migrate InspectorDOMAgent to new style

[DevTools] migrate InspectorDOMAgent to new style

BUG=none
R=dgozman@chromium.org

Committed: https://crrev.com/6d24b35ab8fe401d4da208873818958d5a4868be
Cr-Commit-Position: refs/heads/master@{#429463}

[kozy, 2016-11-01 15:17:55]

Dmitry, please take a look.

[kozy, 2016-11-01 15:18:42]

Description was changed from

==========
[inspector_protocol] migrate DOM agent to new_style

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

to

==========
[DevTools] migrate DOM agent to new_style

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

[kozy, 2016-11-01 15:57:26]

Description was changed from

==========
[DevTools] migrate DOM agent to new_style

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

to

==========
[DevTools] migrate InspectorDOMAgent to new style

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

[dgozman, 2016-11-01 18:36:31]

You'll have to migrate ash devtools as well (they reuse DOM agent).

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

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/DOMEditor.cpp:449: static Response
populateErrorString(ExceptionState& exceptionState) {
InspectorDOMAgent::toResponse

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/DOMEditor.cpp:451: return
Response::Error(DOMException::getErrorName(exceptionState.code()));
Add a message here (it's done in some other agents).

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

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:481: Response
response = setSearchingForNode(
Ignore the result.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:487: return
response;
return Response::OK();

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:620: return
Response::Error("Not a container node");
Nice!

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:781: return
m_domEditor->removeAttribute(element, caseAdjustedName);
Add {} around return.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:814: return
Response::Error("Node with passed nodeId not found");
Maybe use assertElement here?

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:819: if
(exceptionState.hadException())
Use toResponse(exceptionState)

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1289: if
(!(response = assertNode(nodeId.fromJust(), node)).isSuccess())
You can just assign to response and check of success below (instead of !node
check).

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1356: Response
response = assertEditableChildNode(
Reuse |response| variable.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1397: Response
response = assertEditableChildNode(
ditto

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1520: Node* node;
= nullptr;

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1523: *nodeId =
0;
No need to zero it when returning non-success.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/InspectorDOMAgent.h (right):

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.h:267: Response
assertDocument(int nodeId, Document*&);
This one is never used - remove it.

[kozy, 2016-11-02 16:06:04]

All done, please take a look.

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

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/DOMEditor.cpp:449: static Response
populateErrorString(ExceptionState& exceptionState) {
On 2016/11/01 18:36:30, dgozman wrote:
> InspectorDOMAgent::toResponse

Separate copy since this file doesn't include InspectorDOMAgent.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/DOMEditor.cpp:451: return
Response::Error(DOMException::getErrorName(exceptionState.code()));
On 2016/11/01 18:36:30, dgozman wrote:
> Add a message here (it's done in some other agents).

Done.

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

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:481: Response
response = setSearchingForNode(
On 2016/11/01 18:36:31, dgozman wrote:
> Ignore the result.

Done.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:487: return
response;
On 2016/11/01 18:36:31, dgozman wrote:
> return Response::OK();

Done.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:620: return
Response::Error("Not a container node");
On 2016/11/01 18:36:31, dgozman wrote:
> Nice!

Done.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:781: return
m_domEditor->removeAttribute(element, caseAdjustedName);
On 2016/11/01 18:36:31, dgozman wrote:
> Add {} around return.

Done.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:814: return
Response::Error("Node with passed nodeId not found");
On 2016/11/01 18:36:30, dgozman wrote:
> Maybe use assertElement here?

Done.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:819: if
(exceptionState.hadException())
On 2016/11/01 18:36:31, dgozman wrote:
> Use toResponse(exceptionState)

Done.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1289: if
(!(response = assertNode(nodeId.fromJust(), node)).isSuccess())
On 2016/11/01 18:36:31, dgozman wrote:
> You can just assign to response and check of success below (instead of !node
> check).

Done.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1356: Response
response = assertEditableChildNode(
On 2016/11/01 18:36:30, dgozman wrote:
> Reuse |response| variable.

Done.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1397: Response
response = assertEditableChildNode(
On 2016/11/01 18:36:31, dgozman wrote:
> ditto

Done.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1520: Node* node;
On 2016/11/01 18:36:30, dgozman wrote:
> = nullptr;

Done.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1523: *nodeId =
0;
On 2016/11/01 18:36:31, dgozman wrote:
> No need to zero it when returning non-success.

Done.

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/InspectorDOMAgent.h (right):

https://codereview.chromium.org/2468803002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.h:267: Response
assertDocument(int nodeId, Document*&);
On 2016/11/01 18:36:31, dgozman wrote:
> This one is never used - remove it.

Done.

[kozy, 2016-11-02 16:40:22]

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

[kozy, 2016-11-02 16:40:22]

Sadrul, please take a look.
I need owner lgtm for ash/devtools.

[kozy, 2016-11-02 16:41:49]

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

[kozy, 2016-11-02 16:41:49]

Alice, please take a look.
I need owner lgtm for InspectorAccessibilityAgent.

[kozy, 2016-11-02 16:43:01]

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

[commit-bot: I haz the power, 2016-11-02 16:43:27]

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

[sadrul, 2016-11-02 17:00:04]

stamp lgtm for ash changes. (cc +mhashmi@)

[dgozman, 2016-11-02 17:06:41]

lgtm

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

https://codereview.chromium.org/2468803002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1281: response =
!node ? Response::InternalError() : Response::OK();
InternalError -> Error("No node found for given backend id")

[commit-bot: I haz the power, 2016-11-02 18:14:22]

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

[commit-bot: I haz the power, 2016-11-02 18:14:23]

Dry run: This issue passed the CQ dry run.

[kozy, 2016-11-02 18:37:28]

All done, thanks!

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

https://codereview.chromium.org/2468803002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1281: response =
!node ? Response::InternalError() : Response::OK();
On 2016/11/02 17:06:41, dgozman wrote:
> InternalError -> Error("No node found for given backend id")

Done.

[kozy, 2016-11-02 21:55:55]

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

[commit-bot: I haz the power, 2016-11-02 21:56:37]

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

[aboxhall, 2016-11-02 22:23:43]

lgtm for InspectorAccessibilityAgent :D

[commit-bot: I haz the power, 2016-11-02 23:38:13]

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

[commit-bot: I haz the power, 2016-11-02 23:38:14]

Dry run: This issue passed the CQ dry run.

[kozy, 2016-11-02 23:54:48]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2016-11-02 23:54:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, sadrul@chromium.org
Link to the patchset: https://codereview.chromium.org/2468803002/#ps80001
(title: "addressed comments and rebased")

[commit-bot: I haz the power, 2016-11-02 23:55:09]

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-11-03 00:00:52]

Description was changed from

==========
[DevTools] migrate InspectorDOMAgent to new style

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

to

==========
[DevTools] migrate InspectorDOMAgent to new style

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

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

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-03 00:04:08]

Description was changed from

==========
[DevTools] migrate InspectorDOMAgent to new style

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

to

==========
[DevTools] migrate InspectorDOMAgent to new style

BUG=none
R=dgozman@chromium.org

Committed: https://crrev.com/6d24b35ab8fe401d4da208873818958d5a4868be
Cr-Commit-Position: refs/heads/master@{#429463}
==========

[commit-bot: I haz the power, 2016-11-03 00:04:09]

Patchset 5 (id:??) landed as
https://crrev.com/6d24b35ab8fe401d4da208873818958d5a4868be
Cr-Commit-Position: refs/heads/master@{#429463}