Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(448)

Issue 2633343003: Adds DOM.getFlatDocument which returns an array of nodes (Closed)

Created:
3 years, 11 months ago by alex clarke (OOO till 29th)
Modified:
3 years, 10 months ago
Reviewers:
Sami, pfeldman
CC:
chromium-reviews, caseq+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, blink-reviews, kozyatinskiy+blink_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Adds DOM.getFlatDocument which returns an array of nodes We need this because some web pages have very deep dom trees which causes the JSON parser to CHECK (there's currently a limit of 200 nodes). BUG=546953 Review-Url: https://codereview.chromium.org/2633343003 Cr-Commit-Position: refs/heads/master@{#447844} Committed: https://chromium.googlesource.com/chromium/src/+/ea635266172221937f38f16417c0b622a5bdef97

Patch Set 1 #

Total comments: 11

Patch Set 2 : Addressed some of the revire comments (just want to see if the test works cross platform now). #

Patch Set 3 : Try to share code #

Total comments: 2

Patch Set 4 : Removed visitor #

Total comments: 2

Patch Set 5 : new-style #

Patch Set 6 : Fix layout test #

Total comments: 10

Patch Set 7 : Changes for Pavel #

Patch Set 8 : Fix headless browser test #

Messages

Total messages: 53 (33 generated)
alex clarke (OOO till 29th)
3 years, 11 months ago (2017-01-17 15:47:59 UTC) #3
Sami
https://codereview.chromium.org/2633343003/diff/1/third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getFlatDocument-expected.txt File third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getFlatDocument-expected.txt (right): https://codereview.chromium.org/2633343003/diff/1/third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getFlatDocument-expected.txt#newcode414 third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getFlatDocument-expected.txt:414: "documentURL": "file:///usr/local/google/code/clankium/src/third_party/WebKit/LayoutTests/inspector-protocol/dom/resources/simple-iframe.html", Do we need to strip these out? ...
3 years, 11 months ago (2017-01-17 17:57:43 UTC) #7
pfeldman
https://codereview.chromium.org/2633343003/diff/1/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp File third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp (right): https://codereview.chromium.org/2633343003/diff/1/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp#newcode1728 third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1728: switch (node->getNodeType()) { Can we extract some (all) code ...
3 years, 11 months ago (2017-01-17 18:34:58 UTC) #9
alex clarke (OOO till 29th)
PTAL https://codereview.chromium.org/2633343003/diff/1/third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getFlatDocument-expected.txt File third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getFlatDocument-expected.txt (right): https://codereview.chromium.org/2633343003/diff/1/third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getFlatDocument-expected.txt#newcode414 third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getFlatDocument-expected.txt:414: "documentURL": "file:///usr/local/google/code/clankium/src/third_party/WebKit/LayoutTests/inspector-protocol/dom/resources/simple-iframe.html", On 2017/01/17 17:57:43, Sami wrote: > ...
3 years, 11 months ago (2017-01-18 16:31:08 UTC) #14
Sami
Thanks! Non-owner lgtm. https://codereview.chromium.org/2633343003/diff/1/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp File third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp (right): https://codereview.chromium.org/2633343003/diff/1/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp#newcode512 third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:512: int sanitizedDepth = depth.fromMaybe(2); On 2017/01/18 ...
3 years, 11 months ago (2017-01-18 18:10:46 UTC) #17
pfeldman
https://codereview.chromium.org/2633343003/diff/40001/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp File third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp (right): https://codereview.chromium.org/2633343003/diff/40001/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp#newcode1616 third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1616: std::unique_ptr<protocol::DOM::Node> InspectorDOMAgent::buildObjectForNode( Could we simplify things via adding an ...
3 years, 11 months ago (2017-01-18 22:48:20 UTC) #20
alex clarke (OOO till 29th)
PTAL https://codereview.chromium.org/2633343003/diff/40001/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp File third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp (right): https://codereview.chromium.org/2633343003/diff/40001/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp#newcode1616 third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1616: std::unique_ptr<protocol::DOM::Node> InspectorDOMAgent::buildObjectForNode( On 2017/01/18 22:48:20, pfeldman wrote: > ...
3 years, 11 months ago (2017-01-19 10:26:41 UTC) #22
pfeldman
https://codereview.chromium.org/2633343003/diff/60001/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp File third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp (right): https://codereview.chromium.org/2633343003/diff/60001/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp#newcode1815 third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1815: children->addItem(minimalCopy(flattenResult->get(nextIndex))); i don't follow the minimalCopy bit. I was ...
3 years, 11 months ago (2017-01-19 21:26:00 UTC) #26
alex clarke (OOO till 29th)
https://codereview.chromium.org/2633343003/diff/60001/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp File third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp (right): https://codereview.chromium.org/2633343003/diff/60001/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp#newcode1815 third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:1815: children->addItem(minimalCopy(flattenResult->get(nextIndex))); On 2017/01/19 21:25:59, pfeldman wrote: > i don't ...
3 years, 11 months ago (2017-01-20 13:48:28 UTC) #27
pfeldman
> The reason I added this is to disambiguate the various types of sub node: ...
3 years, 11 months ago (2017-01-20 20:27:16 UTC) #28
alex clarke (OOO till 29th)
On 2017/01/20 20:27:16, pfeldman wrote: > > The reason I added this is to disambiguate ...
3 years, 11 months ago (2017-01-26 16:16:30 UTC) #29
pfeldman
> Whoops I meant to reply to this. How do we know the parent ID ...
3 years, 11 months ago (2017-01-26 18:15:13 UTC) #30
alex clarke (OOO till 29th)
On 2017/01/26 18:15:13, pfeldman wrote: > > Whoops I meant to reply to this. How ...
3 years, 11 months ago (2017-01-26 22:42:43 UTC) #31
pfeldman
> { > "id": "FlatNode", > "type": "object", > "properties": [ > { "name": "node", ...
3 years, 10 months ago (2017-01-30 18:46:34 UTC) #32
alex clarke (OOO till 29th)
On 2017/01/30 18:46:34, pfeldman wrote: > > { > > "id": "FlatNode", > > "type": ...
3 years, 10 months ago (2017-02-01 21:46:22 UTC) #39
pfeldman
lgtm % comments. https://codereview.chromium.org/2633343003/diff/100001/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp File third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp (right): https://codereview.chromium.org/2633343003/diff/100001/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp#newcode502 third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:502: if (!enabled()) You can return error ...
3 years, 10 months ago (2017-02-02 01:24:19 UTC) #40
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2633343003/120001
3 years, 10 months ago (2017-02-02 17:52:02 UTC) #46
alex clarke (OOO till 29th)
Thanks https://codereview.chromium.org/2633343003/diff/100001/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp File third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp (right): https://codereview.chromium.org/2633343003/diff/100001/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp#newcode502 third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:502: if (!enabled()) On 2017/02/02 01:24:19, pfeldman wrote: > ...
3 years, 10 months ago (2017-02-02 17:53:07 UTC) #47
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2633343003/140001
3 years, 10 months ago (2017-02-02 19:36:04 UTC) #50
commit-bot: I haz the power
3 years, 10 months ago (2017-02-02 21:25:38 UTC) #53
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/ea635266172221937f38f16417c0...

Powered by Google App Engine
This is Rietveld 408576698