Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2346853002:
Add a DOM.getLayoutTreeNodes devtools command (Closed)
|
Created:
4 years, 3 months ago by alex clarke (OOO till 29th) Modified:
4 years, 2 months ago CC:
apavlov+blink_chromium.org, blink-reviews, caseq+blink_chromium.org, chromium-reviews, devtools-reviews_chromium.org, kozyatinskiy+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd a DOM.getLayoutTreeNodes devtools command
This command will help Headless API users understand the layout
of the page.
BUG=546953
Committed: https://crrev.com/8a9aad4ec3fcf29e289898802b7f2fd43be83398
Cr-Commit-Position: refs/heads/master@{#423623}
Patch Set 1 #Patch Set 2 : Add missing files #Patch Set 3 : Pretty print #
Total comments: 12
Patch Set 4 : Changes for Pavel #
Total comments: 4
Patch Set 5 : Rename to DOM.getLayoutTreeNodes #Patch Set 6 : Canonicalize node ids in the layout test. #Patch Set 7 : Try specify a font size & family to make mac & pc pass #Patch Set 8 : Instead lets have mac and windows expects #Patch Set 9 : Try the Ahem font #Patch Set 10 : Make it work with iframes #
Total comments: 12
Patch Set 11 : Changes for Elliott #
Total comments: 4
Patch Set 12 : Documentation nits #Patch Set 13 : Rebased #Patch Set 14 : Rebased #Patch Set 15 : Fix iframe font path #Messages
Total messages: 81 (55 generated)
The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
alexclarke@chromium.org changed reviewers: + pfeldman@chromium.org, skyostil@chromium.org
PTAL
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
https://codereview.chromium.org/2346853002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp (right): https://codereview.chromium.org/2346853002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:534: std::unique_ptr<protocol::DOM::Rect> getRectForFloatRect(const FloatRect& rect) We call these build*. Also, either move those to the top of the file or drop the namespace and use static modifiers. https://codereview.chromium.org/2346853002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:544: FloatRect getAbsoluteBoundingBox(LayoutObject* layoutObject) Blink does not (yet) use get prefixes. https://codereview.chromium.org/2346853002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:548: return layoutObject->localToAbsoluteQuad(localRect).boundingBox(); I am not sure this matches your expectations wrt coordinate system, but you'll know better once you apply it. Make sure you checked it for the iframes, scrolled iframes, etc. https://codereview.chromium.org/2346853002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:574: if (!layoutObject) You still might want to go over the dom hierarchy. https://codereview.chromium.org/2346853002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:578: DCHECK(backendNodeId); It is always there https://codereview.chromium.org/2346853002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:605: for (node = innerFirstChild(node); node; node = innerNextSibling(node)) I don't see where you account for the shadow dom. Also, this seems to be an array of layout objects for nodes rather than the render tree. Didn't you want to traverse the render tree instead?
PTAL. PS any idea why I'm getting different node IDs for mac? https://codereview.chromium.org/2346853002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp (right): https://codereview.chromium.org/2346853002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:534: std::unique_ptr<protocol::DOM::Rect> getRectForFloatRect(const FloatRect& rect) On 2016/09/16 00:59:35, pfeldman wrote: > We call these build*. Also, either move those to the top of the file or drop the > namespace and use static modifiers. Done. https://codereview.chromium.org/2346853002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:544: FloatRect getAbsoluteBoundingBox(LayoutObject* layoutObject) On 2016/09/16 00:59:35, pfeldman wrote: > Blink does not (yet) use get prefixes. Done. https://codereview.chromium.org/2346853002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:548: return layoutObject->localToAbsoluteQuad(localRect).boundingBox(); On 2016/09/16 00:59:35, pfeldman wrote: > I am not sure this matches your expectations wrt coordinate system, but you'll > know better once you apply it. Make sure you checked it for the iframes, > scrolled iframes, etc. Acknowledged. https://codereview.chromium.org/2346853002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:574: if (!layoutObject) On 2016/09/16 00:59:35, pfeldman wrote: > You still might want to go over the dom hierarchy. Good point! https://codereview.chromium.org/2346853002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:578: DCHECK(backendNodeId); On 2016/09/16 00:59:35, pfeldman wrote: > It is always there Done. https://codereview.chromium.org/2346853002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:605: for (node = innerFirstChild(node); node; node = innerNextSibling(node)) On 2016/09/16 00:59:35, pfeldman wrote: > I don't see where you account for the shadow dom. Hopefully I've done that now. > Also, this seems to be an > array of layout objects for nodes rather than the render tree. Didn't you want > to traverse the render tree instead? I'm under the impression that we don't want to traverse the render tree. Perhaps we should discuss this offline?
The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getRenderTreeNodes.html (right): https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getRenderTreeNodes.html:35: <div class="class5 class6"></div> As one more sanity check could you add a div with a "transform: rotateZ(90deg)" to make sure we're getting a tall bounding box instead of a wide one? https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/inspector/browser_protocol.json (right): https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/browser_protocol.json:2175: "name": "getRenderTreeNodes", Should we call this getLayoutTreeNodes instead to avoid the confusion Pavel mentioned? We could think about getting the render (paint) tree later but it doesn't seem necessary now.
On 2016/09/16 12:31:59, Sami (OoO) wrote: > https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Layo... > File > third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getRenderTreeNodes.html > (right): > > https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getRenderTreeNodes.html:35: > <div class="class5 class6"></div> > As one more sanity check could you add a div with a "transform: rotateZ(90deg)" > to make sure we're getting a tall bounding box instead of a wide one? > > https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/inspector/browser_protocol.json (right): > > https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/inspector/browser_protocol.json:2175: "name": > "getRenderTreeNodes", > Should we call this getLayoutTreeNodes instead to avoid the confusion Pavel > mentioned? We could think about getting the render (paint) tree later but it > doesn't seem necessary now. How would the client use your backend ids? Resolve them on-by-one? If what you are after is box models for the DOM nodes, should it be a parameter on "requestChildNodes" that would make nodes arrive with the "box" property?
On 2016/09/19 22:58:23, pfeldman wrote: > On 2016/09/16 12:31:59, Sami (OoO) wrote: > > > https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Layo... > > File > > > third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getRenderTreeNodes.html > > (right): > > > > > https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Layo... > > > third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getRenderTreeNodes.html:35: > > <div class="class5 class6"></div> > > As one more sanity check could you add a div with a "transform: > rotateZ(90deg)" > > to make sure we're getting a tall bounding box instead of a wide one? > > > > > https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Sour... > > File third_party/WebKit/Source/core/inspector/browser_protocol.json (right): > > > > > https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Sour... > > third_party/WebKit/Source/core/inspector/browser_protocol.json:2175: "name": > > "getRenderTreeNodes", > > Should we call this getLayoutTreeNodes instead to avoid the confusion Pavel > > mentioned? We could think about getting the render (paint) tree later but it > > doesn't seem necessary now. > > How would the client use your backend ids? Resolve them on-by-one? If what you > are after is box models for the DOM nodes, should it be a parameter on > "requestChildNodes" that would make nodes arrive with the "box" property? An example usage would be to retrieve all DOM nodes, and then use this new command to get the bounding boxes (in absolute coordinates) and the render text details. Note currently to currently retrieve the whole DOM is kinda complicated and (separately?) we'd be interested in adding a command (or a parameter to an existing one) to get everything in one go.
Description was changed from ========== Add a DOM.getRenderTreeNodes devtools command This command will help Headless API users understand the layout of the page. BUG=546953 ========== to ========== Add a DOM.getLayoutTreeNodes devtools command This command will help Headless API users understand the layout of the page. BUG=546953 ==========
PTAL https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getRenderTreeNodes.html (right): https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/inspector-protocol/dom/dom-getRenderTreeNodes.html:35: <div class="class5 class6"></div> On 2016/09/16 12:31:59, Sami (OoO) wrote: > As one more sanity check could you add a div with a "transform: rotateZ(90deg)" > to make sure we're getting a tall bounding box instead of a wide one? Done, the node with the 90 deg rotation has a tall thin bounding box as expected. { "backendNodeId": 7, "boundingBox": { "x": 99, "y": 226, "width": 19, "height": 79 }, "layoutText": "Rotated text!", "inlineTextNodes": [ { "boundingBox": { "x": 99, "y": 226, "width": 19, "height": 79 }, "text": "Rotated text!" } ] }, https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/inspector/browser_protocol.json (right): https://codereview.chromium.org/2346853002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/inspector/browser_protocol.json:2175: "name": "getRenderTreeNodes", Done
The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
But you should be able to get everything in one go - there was a depth parameter in the requestChildNodes
On 2016/09/20 23:09:21, pfeldman wrote: > But you should be able to get everything in one go - there was a depth parameter > in the requestChildNodes Not quite, to get everything across iframe boundaries you have to: 1. Start observing DOM events 2. Issue a DOM.getDocument command (I wish this could get everything) 3. Then issue a DOM.requestChildNodes with a depth of -1 4. For any iframe nodes issue a subsequent DOM.requestChildNodes 5. Stop observing DOM events If any JS modifies the DOM during this process you might get unexpected DOM.setChildNodes events.
The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
Right, but we already suffer from these when working with DOM, so working it around for the boxes would not be a good design. We could make -1 pierce the frame boundary.
The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
alexclarke@chromium.org changed reviewers: + esprehn@chromium.org
The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
We can do this API, but I don't think we want to support the shape of the inline boxes as a stable API. So the consumers of headless should not assume that we'll keep the same number of boxes for the given input between chrome versions. Anything that's not getClientRects() is not stable API. https://codereview.chromium.org/2346853002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp (right): https://codereview.chromium.org/2346853002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:151: FloatRect buildAbsoluteBoundingBox(LayoutObject* layoutObject) I think you want to use boundsInViewport() from the DOM instead? This function is using the very low level stuff, and feels like duplication. https://codereview.chromium.org/2346853002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:566: std::vector<Node*> unvisited; // Neither WTF::Vector or HeapVector allow Node* :( HeapVector<Member<Node>>, you probably want a big inline capacity too to avoid the malloc cost https://codereview.chromium.org/2346853002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:569: while (!unvisited.empty()) { this is very strange to put everything into a vector, just iterate the document with Traversal. https://codereview.chromium.org/2346853002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:606: for (const InlineTextBox* itb = layoutText->firstTextBox(); itb; itb = itb->nextTextBox()) { box or textBox https://codereview.chromium.org/2346853002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:608: FloatRect absoluteItbRect = layoutObject->localToAbsoluteQuad(localItbRect).boundingBox(); what is Itb? Don't abbreciate in blink https://codereview.chromium.org/2346853002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:611: .setText(layoutText->text().substring(itb->start(), itb->len()).utf8().data()) this is allocating tons of strings and will be slow, you do 3 string copies in here. Also accessing the low level API like start() and len() is unfortunate, we're trying to get people to stop doing that. :)
The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run
PTAL https://codereview.chromium.org/2346853002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp (right): https://codereview.chromium.org/2346853002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:151: FloatRect buildAbsoluteBoundingBox(LayoutObject* layoutObject) On 2016/09/28 04:02:49, esprehn wrote: > I think you want to use boundsInViewport() from the DOM instead? This function > is using the very low level stuff, and feels like duplication. Done. https://codereview.chromium.org/2346853002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:566: std::vector<Node*> unvisited; // Neither WTF::Vector or HeapVector allow Node* :( On 2016/09/28 04:02:49, esprehn wrote: > HeapVector<Member<Node>>, you probably want a big inline capacity too to avoid > the malloc cost Done. https://codereview.chromium.org/2346853002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:569: while (!unvisited.empty()) { On 2016/09/28 04:02:49, esprehn wrote: > this is very strange to put everything into a vector, just iterate the document > with Traversal. Done. https://codereview.chromium.org/2346853002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:606: for (const InlineTextBox* itb = layoutText->firstTextBox(); itb; itb = itb->nextTextBox()) { On 2016/09/28 04:02:49, esprehn wrote: > box or textBox Done. https://codereview.chromium.org/2346853002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:608: FloatRect absoluteItbRect = layoutObject->localToAbsoluteQuad(localItbRect).boundingBox(); On 2016/09/28 04:02:49, esprehn wrote: > what is Itb? Don't abbreciate in blink Done. https://codereview.chromium.org/2346853002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:611: .setText(layoutText->text().substring(itb->start(), itb->len()).utf8().data()) On 2016/09/28 04:02:49, esprehn wrote: > this is allocating tons of strings and will be slow, you do 3 string copies in > here. Also accessing the low level API like start() and len() is unfortunate, > we're trying to get people to stop doing that. :) We don't really need the substring, the offset and length suffice although the user has to be cautious since the units are characters not bytes.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
ping :)
lgtm. https://codereview.chromium.org/2346853002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/inspector/browser_protocol.json (right): https://codereview.chromium.org/2346853002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/browser_protocol.json:2071: "description": "Details of post layout rendered text positions. Note layout may change between ", Cut off sentence? https://codereview.chromium.org/2346853002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/browser_protocol.json:2196: "description": "Returns the document's LayoutTreeNodes to the caller.", Maybe mention this includes child documents too?
https://codereview.chromium.org/2346853002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/inspector/browser_protocol.json (right): https://codereview.chromium.org/2346853002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/browser_protocol.json:2071: "description": "Details of post layout rendered text positions. Note layout may change between ", On 2016/09/30 10:05:23, Sami wrote: > Cut off sentence? Done. https://codereview.chromium.org/2346853002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/browser_protocol.json:2196: "description": "Returns the document's LayoutTreeNodes to the caller.", On 2016/09/30 10:05:23, Sami wrote: > Maybe mention this includes child documents too? Done.
The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
Thanks all!
The CQ bit was checked by alexclarke@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from skyostil@chromium.org Link to the patchset: https://codereview.chromium.org/2346853002/#ps220001 (title: "Documentation nits")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:
While running git apply --index -3 -p1;
error: patch failed:
third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:136
Falling back to three-way merge...
Applied patch to
'third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp' with conflicts.
U third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp
Patch: third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp
Index: third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp
diff --git a/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp
b/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp
index
0ceb294c59525ae7a89873664d29373976394f87..2cb8bdd9ac7cca042a6dd44f426b68c43679e8f8
100644
--- a/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp
+++ b/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp
@@ -67,7 +67,9 @@
#include "core/inspector/InspectorHistory.h"
#include "core/inspector/V8InspectorString.h"
#include "core/layout/HitTestResult.h"
+#include "core/layout/LayoutInline.h"
#include "core/layout/api/LayoutViewItem.h"
+#include "core/layout/line/InlineTextBox.h"
#include "core/loader/DocumentLoader.h"
#include "core/page/FrameTree.h"
#include "core/page/Page.h"
@@ -136,8 +138,19 @@ v8::Local<v8::Value> nodeV8Value(v8::Local<v8::Context>
context, Node* node)
return toV8(node, context->Global(), isolate);
}
+std::unique_ptr<protocol::DOM::Rect> buildRectForFloatRect(const FloatRect&
rect)
+{
+ return protocol::DOM::Rect::create()
+ .setX(rect.x())
+ .setY(rect.y())
+ .setWidth(rect.width())
+ .setHeight(rect.height())
+ .build();
+}
+
} // namespace
+
class InspectorRevalidateDOMTask final : public
GarbageCollectedFinalized<InspectorRevalidateDOMTask> {
public:
explicit InspectorRevalidateDOMTask(InspectorDOMAgent*);
@@ -529,6 +542,61 @@ void InspectorDOMAgent::getDocument(ErrorString*
errorString, std::unique_ptr<pr
*root = buildObjectForNode(m_document.get(), 2,
m_documentNodeToIdMap.get());
}
+void InspectorDOMAgent::getLayoutTreeNodes(ErrorString* errorString,
std::unique_ptr<protocol::Array<protocol::DOM::LayoutTreeNode>>*
layoutTreeNodes)
+{
+ layoutTreeNodes->reset(new protocol::Array<protocol::DOM::LayoutTreeNode>);
+ visitLayoutTreeNodes(m_document.get(), *layoutTreeNodes->get());
+}
+
+void InspectorDOMAgent::visitLayoutTreeNodes(Node* node,
protocol::Array<protocol::DOM::LayoutTreeNode>& layoutTreeNodes)
+{
+ for (; node; node = NodeTraversal::next(*node)) {
+ // Visit shadow dom nodes.
+ if (node->isElementNode()) {
+ const Element* element = toElement(node);
+ ElementShadow* elementShadow = element->shadow();
+ if (elementShadow)
+ visitLayoutTreeNodes(&elementShadow->youngestShadowRoot(),
layoutTreeNodes);
+ }
+
+ // Pierce iframe boundaries.
+ if (node->isFrameOwnerElement())
+
visitLayoutTreeNodes(toHTMLFrameOwnerElement(node)->contentDocument()->documentElement(),
layoutTreeNodes);
+
+ LayoutObject* layoutObject = node->layoutObject();
+ if (!layoutObject)
+ continue;
+
+ int backendNodeId = DOMNodeIds::idForNode(node);
+ std::unique_ptr<protocol::DOM::LayoutTreeNode> layoutTreeNode =
+ protocol::DOM::LayoutTreeNode::create()
+ .setBackendNodeId(backendNodeId)
+ .setBoundingBox(buildRectForFloatRect(node->isElementNode() ?
FloatRect(toElement(node)->boundsInViewport()) :
layoutObject->absoluteBoundingBoxRect()))
+ .build();
+
+ if (layoutObject->isText()) {
+ LayoutText* layoutText = toLayoutText(layoutObject);
+ layoutTreeNode->setLayoutText(layoutText->text());
+ if (layoutText->hasTextBoxes()) {
+ std::unique_ptr<protocol::Array<protocol::DOM::InlineTextBox>>
inlineTextNodes(new protocol::Array<protocol::DOM::InlineTextBox>());
+ for (const InlineTextBox* textBox = layoutText->firstTextBox();
textBox; textBox = textBox->nextTextBox()) {
+ FloatRect
localCoordsTextBoxRect(textBox->calculateBoundaries());
+ FloatRect absoluteCoordsTextBoxRect =
layoutObject->localToAbsoluteQuad(localCoordsTextBoxRect).boundingBox();
+ inlineTextNodes->addItem(
+ protocol::DOM::InlineTextBox::create()
+ .setStartCharacterIndex(textBox->start())
+ .setNumCharacters(textBox->len())
+
.setBoundingBox(buildRectForFloatRect(absoluteCoordsTextBoxRect))
+ .build());
+ }
+ layoutTreeNode->setInlineTextNodes(std::move(inlineTextNodes));
+ }
+ }
+
+ layoutTreeNodes.addItem(std::move(layoutTreeNode));
+ }
+}
+
void InspectorDOMAgent::pushChildNodesToFrontend(int nodeId, int depth)
{
Node* node = nodeForId(nodeId);
The CQ bit was checked by alexclarke@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from skyostil@chromium.org, pfeldman@chromium.org Link to the patchset: https://codereview.chromium.org/2346853002/#ps240001 (title: "Rebased")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by alexclarke@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from skyostil@chromium.org, pfeldman@chromium.org Link to the patchset: https://codereview.chromium.org/2346853002/#ps280001 (title: "Fix iframe font path")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add a DOM.getLayoutTreeNodes devtools command This command will help Headless API users understand the layout of the page. BUG=546953 ========== to ========== Add a DOM.getLayoutTreeNodes devtools command This command will help Headless API users understand the layout of the page. BUG=546953 ==========
Message was sent while issue was closed.
Committed patchset #15 (id:280001)
Message was sent while issue was closed.
Description was changed from ========== Add a DOM.getLayoutTreeNodes devtools command This command will help Headless API users understand the layout of the page. BUG=546953 ========== to ========== Add a DOM.getLayoutTreeNodes devtools command This command will help Headless API users understand the layout of the page. BUG=546953 Committed: https://crrev.com/8a9aad4ec3fcf29e289898802b7f2fd43be83398 Cr-Commit-Position: refs/heads/master@{#423623} ==========
Message was sent while issue was closed.
Patchset 15 (id:??) landed as https://crrev.com/8a9aad4ec3fcf29e289898802b7f2fd43be83398 Cr-Commit-Position: refs/heads/master@{#423623} |
