Add a utility class for extracting details of the DOM

Add a utility class for extracting details of the DOM, including
white-listed computed styles.

BUG=546953
Committed: https://crrev.com/03139e4a4f3d029eb09c5fa831f0307e4fb1911b
Cr-Commit-Position: refs/heads/master@{#426442}

[alex clarke (OOO till 29th), 2016-09-30 10:18:52]

alexclarke@chromium.org changed reviewers:
+ skyostil@chromium.org

[alex clarke (OOO till 29th), 2016-09-30 10:18:53]

PTAL

[Sami, 2016-09-30 10:56:03]

Nice.

https://codereview.chromium.org/2385653003/diff/1/headless/public/util/dom_tr...
File headless/public/util/dom_tree_extractor.cc (right):

https://codereview.chromium.org/2385653003/diff/1/headless/public/util/dom_tr...
headless/public/util/dom_tree_extractor.cc:47:
devtools_client_->GetDOM()->AddObserver(this);
Probably should avoid doing this twice?

https://codereview.chromium.org/2385653003/diff/1/headless/public/util/dom_tr...
headless/public/util/dom_tree_extractor.cc:120:
dom_nodes[i]->Set("childIndicies", std::move(children));
typo: indices

https://codereview.chromium.org/2385653003/diff/1/headless/public/util/dom_tr...
headless/public/util/dom_tree_extractor.cc:134: // Merge in Render Tree.
nit: layout tree

https://codereview.chromium.org/2385653003/diff/1/headless/public/util/dom_tr...
File headless/public/util/dom_tree_extractor.h (right):

https://codereview.chromium.org/2385653003/diff/1/headless/public/util/dom_tr...
headless/public/util/dom_tree_extractor.h:20: // notice).
Could you briefly document what the received structure looks like?

[alex clarke (OOO till 29th), 2016-09-30 13:16:33]

As discussed offline I changed what gets returned to avoid a bunch of work
Serializing.  The test now does that and also pretty prints the dictionaries
which hopefully makes them easier to read.

https://codereview.chromium.org/2385653003/diff/1/headless/public/util/dom_tr...
File headless/public/util/dom_tree_extractor.cc (right):

https://codereview.chromium.org/2385653003/diff/1/headless/public/util/dom_tr...
headless/public/util/dom_tree_extractor.cc:47:
devtools_client_->GetDOM()->AddObserver(this);
On 2016/09/30 10:56:03, Sami wrote:
> Probably should avoid doing this twice?

Done.

https://codereview.chromium.org/2385653003/diff/1/headless/public/util/dom_tr...
headless/public/util/dom_tree_extractor.cc:120:
dom_nodes[i]->Set("childIndicies", std::move(children));
On 2016/09/30 10:56:03, Sami wrote:
> typo: indices

Done.

https://codereview.chromium.org/2385653003/diff/1/headless/public/util/dom_tr...
headless/public/util/dom_tree_extractor.cc:134: // Merge in Render Tree.
On 2016/09/30 10:56:03, Sami wrote:
> nit: layout tree

Done.

https://codereview.chromium.org/2385653003/diff/1/headless/public/util/dom_tr...
File headless/public/util/dom_tree_extractor.h (right):

https://codereview.chromium.org/2385653003/diff/1/headless/public/util/dom_tr...
headless/public/util/dom_tree_extractor.h:20: // notice).
On 2016/09/30 10:56:03, Sami wrote:
> Could you briefly document what the received structure looks like?

Is that still needed? It should be obvious now.

[alex clarke (OOO till 29th), 2016-09-30 13:22:59]

As discussed offline I changed what gets returned to avoid a bunch of work
Serializing.  The test now does that and also pretty prints the dictionaries
which hopefully makes them easier to read.

[Sami, 2016-10-03 10:42:19]

lgtm.

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
File headless/public/util/dom_tree_extractor.cc (right):

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
headless/public/util/dom_tree_extractor.cc:32: // (which only contains a few
nodes) and then we fetch all it's children
s/it's/its/

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
headless/public/util/dom_tree_extractor.cc:51:
devtools_client_->GetDOM()->RequestChildNodes(
Curious: does this work on a document that is completely empty besides the root?

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
headless/public/util/dom_tree_extractor.cc:137:
DomTreeExtractor::DomTree::DomTree(DomTree&& other) {
Could this be "= default"?

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
File headless/public/util/dom_tree_extractor.h (right):

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
headless/public/util/dom_tree_extractor.h:36: std::vector<const dom::Node*>
dom_nodes_;
The root (document) is always guaranteed to be first, right?

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
headless/public/util/dom_tree_extractor.h:51: };
DISALLOW_COPY_AND_ASSIGN?

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
headless/public/util/dom_tree_extractor.h:55: // Extracts all nodes from the
DOM.  This is an asynchronous operation and its
s/its/it's/

[alex clarke (OOO till 29th), 2016-10-19 16:23:00]

Description was changed from

==========
Add a utility class for extracting details of the DOM

BUG=546953
==========

to

==========
Add a utility class for extracting details of the DOM, including 
white-listed computed styles.

BUG=546953
==========

[alex clarke (OOO till 29th), 2016-10-19 16:23:10]

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

[alex clarke (OOO till 29th), 2016-10-19 16:23:25]

alexclarke@chromium.org changed reviewers:
+ eseckler@chromium.org

[alex clarke (OOO till 29th), 2016-10-19 16:23:26]

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
File headless/public/util/dom_tree_extractor.cc (right):

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
headless/public/util/dom_tree_extractor.cc:32: // (which only contains a few
nodes) and then we fetch all it's children
On 2016/10/03 10:42:18, Sami (OoO 0x1F1EF 0x1F1F5) wrote:
> s/it's/its/

Acknowledged.

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
headless/public/util/dom_tree_extractor.cc:51:
devtools_client_->GetDOM()->RequestChildNodes(
On 2016/10/03 10:42:18, Sami (OoO 0x1F1EF 0x1F1F5) wrote:
> Curious: does this work on a document that is completely empty besides the
root?

It would have.  Anyway it's gone now, since we can now get everything in one go!

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
headless/public/util/dom_tree_extractor.cc:137:
DomTreeExtractor::DomTree::DomTree(DomTree&& other) {
On 2016/10/03 10:42:18, Sami (OoO 0x1F1EF 0x1F1F5) wrote:
> Could this be "= default"?

Done.

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
File headless/public/util/dom_tree_extractor.h (right):

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
headless/public/util/dom_tree_extractor.h:36: std::vector<const dom::Node*>
dom_nodes_;
On 2016/10/03 10:42:18, Sami (OoO 0x1F1EF 0x1F1F5) wrote:
> The root (document) is always guaranteed to be first, right?

Yes. I've added a comment.

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
headless/public/util/dom_tree_extractor.h:51: };
On 2016/10/03 10:42:18, Sami (OoO 0x1F1EF 0x1F1F5) wrote:
> DISALLOW_COPY_AND_ASSIGN?

Done.

https://codereview.chromium.org/2385653003/diff/20001/headless/public/util/do...
headless/public/util/dom_tree_extractor.h:55: // Extracts all nodes from the
DOM.  This is an asynchronous operation and its
On 2016/10/03 10:42:18, Sami (OoO 0x1F1EF 0x1F1F5) wrote:
> s/its/it's/

Done.

[commit-bot: I haz the power, 2016-10-19 16:23:29]

Dry run: 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-10-19 17:47:43]

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

[commit-bot: I haz the power, 2016-10-19 17:47:44]

Dry run: This issue passed the CQ dry run.

[Eric Seckler, 2016-10-20 08:43:58]

lgtm

https://codereview.chromium.org/2385653003/diff/40001/headless/public/util/do...
File headless/public/util/dom_tree_extractor.h (right):

https://codereview.chromium.org/2385653003/diff/40001/headless/public/util/do...
headless/public/util/dom_tree_extractor.h:19: // addition it also extracts
details of bounding boxes and layout text (NB the
super-nit: addition,

https://codereview.chromium.org/2385653003/diff/40001/headless/public/util/do...
File headless/public/util/dom_tree_extractor_browsertest.cc (right):

https://codereview.chromium.org/2385653003/diff/40001/headless/public/util/do...
headless/public/util/dom_tree_extractor_browsertest.cc:136: for (const auto&
style_proprty :
nit: style_property

[alex clarke (OOO till 29th), 2016-10-20 09:26:25]

All done. Thanks.

https://codereview.chromium.org/2385653003/diff/40001/headless/public/util/do...
File headless/public/util/dom_tree_extractor.h (right):

https://codereview.chromium.org/2385653003/diff/40001/headless/public/util/do...
headless/public/util/dom_tree_extractor.h:19: // addition it also extracts
details of bounding boxes and layout text (NB the
On 2016/10/20 08:43:58, Eric Seckler wrote:
> super-nit: addition,

Done.

https://codereview.chromium.org/2385653003/diff/40001/headless/public/util/do...
File headless/public/util/dom_tree_extractor_browsertest.cc (right):

https://codereview.chromium.org/2385653003/diff/40001/headless/public/util/do...
headless/public/util/dom_tree_extractor_browsertest.cc:136: for (const auto&
style_proprty :
On 2016/10/20 08:43:58, Eric Seckler wrote:
> nit: style_property

Done.

[alex clarke (OOO till 29th), 2016-10-20 09:26:33]

The CQ bit was checked by alexclarke@chromium.org

[alex clarke (OOO till 29th), 2016-10-20 09:26:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
skyostil@chromium.org, eseckler@chromium.org
Link to the patchset: https://codereview.chromium.org/2385653003/#ps60001
(title: "Nits")

[commit-bot: I haz the power, 2016-10-20 09:26:44]

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-10-20 10:16:03]

Description was changed from

==========
Add a utility class for extracting details of the DOM, including 
white-listed computed styles.

BUG=546953
==========

to

==========
Add a utility class for extracting details of the DOM, including 
white-listed computed styles.

BUG=546953
==========

[commit-bot: I haz the power, 2016-10-20 10:16:04]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-21 13:17:02]

Description was changed from

==========
Add a utility class for extracting details of the DOM, including 
white-listed computed styles.

BUG=546953
==========

to

==========
Add a utility class for extracting details of the DOM, including
white-listed computed styles.

BUG=546953

Committed: https://crrev.com/03139e4a4f3d029eb09c5fa831f0307e4fb1911b
Cr-Commit-Position: refs/heads/master@{#426442}
==========

[commit-bot: I haz the power, 2016-10-21 13:17:04]

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