Expand test defaults to include more states, include more blink test results

Expand test defaults to allow more properties by default in Windows and Blink tests.

This is a precursor to including more blink test results in general.

BUG=None

Review-Url: https://codereview.chromium.org/2927033004
Cr-Commit-Position: refs/heads/master@{#479401}
Committed: https://chromium.googlesource.com/chromium/src/+/92bfea769dea215c29270a96fcd56695c3d8c74e

[aleventhal, 2017-06-08 17:22:21]

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

[commit-bot: I haz the power, 2017-06-08 17:22:49]

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, 2017-06-08 18:01:21]

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

[commit-bot: I haz the power, 2017-06-08 18:01:22]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[aleventhal, 2017-06-09 16:37:18]

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

[commit-bot: I haz the power, 2017-06-09 16:37:33]

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, 2017-06-09 17:18:42]

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

[commit-bot: I haz the power, 2017-06-09 17:18:44]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[aleventhal, 2017-06-09 17:49:04]

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

[commit-bot: I haz the power, 2017-06-09 17:49:54]

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, 2017-06-09 18:29:50]

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

[commit-bot: I haz the power, 2017-06-09 18:29:51]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[aleventhal, 2017-06-09 21:36:22]

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

[commit-bot: I haz the power, 2017-06-09 21:36:41]

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, 2017-06-09 22:11:08]

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

[commit-bot: I haz the power, 2017-06-09 22:11:09]

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

[aleventhal, 2017-06-12 14:10:51]

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

[commit-bot: I haz the power, 2017-06-12 14:11:10]

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, 2017-06-12 14:37:46]

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

[commit-bot: I haz the power, 2017-06-12 14:37:47]

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_...)

[aleventhal, 2017-06-12 14:45:26]

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

[commit-bot: I haz the power, 2017-06-12 14:45:39]

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, 2017-06-12 15:55:32]

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

[commit-bot: I haz the power, 2017-06-12 15:55:33]

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_...)

[aleventhal, 2017-06-12 17:21:20]

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

[commit-bot: I haz the power, 2017-06-12 17:21:41]

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, 2017-06-12 18:00:56]

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

[commit-bot: I haz the power, 2017-06-12 18:00:57]

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

[aleventhal, 2017-06-13 12:49:34]

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

[commit-bot: I haz the power, 2017-06-13 12:49:55]

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, 2017-06-13 13:22:08]

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

[commit-bot: I haz the power, 2017-06-13 13:22:10]

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_...)

[aleventhal, 2017-06-13 14:55:21]

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

[commit-bot: I haz the power, 2017-06-13 14:55:38]

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, 2017-06-13 15:29:07]

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

[commit-bot: I haz the power, 2017-06-13 15:29:08]

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

[aleventhal, 2017-06-13 18:12:20]

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

[commit-bot: I haz the power, 2017-06-13 18:13:02]

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, 2017-06-13 18:47:47]

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

[commit-bot: I haz the power, 2017-06-13 18:47:48]

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

[aleventhal, 2017-06-13 19:23:30]

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

[commit-bot: I haz the power, 2017-06-13 19:24:09]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[aleventhal, 2017-06-13 19:26:22]

Description was changed from

==========
Expand test defaults to include more states, include more blink test results

BUG=None
==========

to

==========
Expand test defaults to allow more properties by default in Windows and Blink
tests.

This is a precursor to including more blink test results in general.

BUG=None
==========

[aleventhal, 2017-06-13 19:27:31]

aleventhal@chromium.org changed reviewers:
+ dmazzoni@chromium.org

[aleventhal, 2017-06-13 20:16:55]

Ready for review.

[dmazzoni, 2017-06-13 20:25:34]

lgtm

Great. A few suggestions but essentially no concerns,
this is definitely an improvement.

You can make the final call on these.

https://codereview.chromium.org/2927033004/diff/200001/content/browser/access...
File content/browser/accessibility/dump_accessibility_tree_browsertest.cc
(right):

https://codereview.chromium.org/2927033004/diff/200001/content/browser/access...
content/browser/accessibility/dump_accessibility_tree_browsertest.cc:58: void
AddDefaultFilters(std::vector<Filter>* filters) override {
Since this function is getting long, declare it here and then
define it below the class. That lets you indent by 2 less,
might help wrapping a tiny bit but definitely will make the
class declaration a bit more readable.

https://codereview.chromium.org/2927033004/diff/200001/content/browser/access...
content/browser/accessibility/dump_accessibility_tree_browsertest.cc:63:
filters->push_back(Filter(base::ASCIIToUTF16("ANIMATED*"), Filter::ALLOW));
Do we actually use this?

I'm tempted to get rid of ones we don't actually use *at all*,
or at least group them separately with a comment "not actually
used yet".

https://codereview.chromium.org/2927033004/diff/200001/content/browser/access...
content/browser/accessibility/dump_accessibility_tree_browsertest.cc:73: //
filters->push_back(Filter(base::ASCIIToUTF16("HOTTRACKED"),
Maybe put all of the cpmmented-out off by default ones at the top or bottom.

Feel free to use more whitespace in this function, like:

//
// Windows
//

// Windows states to log by default
filters->push_back(
filters->push_back(
filters->push_back(

// Other Windows attributes

https://codereview.chromium.org/2927033004/diff/200001/content/browser/access...
content/browser/accessibility/dump_accessibility_tree_browsertest.cc:122:
filters->push_back(Filter(base::ASCIIToUTF16("descript*"), Filter::ALLOW));
I vote for either both nameFrom and descriptionFrom, or neither - but having
descriptionFrom but not nameFrom feels wrong

https://codereview.chromium.org/2927033004/diff/200001/content/browser/access...
content/browser/accessibility/dump_accessibility_tree_browsertest.cc:137:
filters->push_back(Filter(base::ASCIIToUTF16("horizontal"), Filter::ALLOW));
This should probably be an orientation enum {horizontal, veritical,
not_applicable}, right?

Does this one show up a lot? I think it might be distracting

https://codereview.chromium.org/2927033004/diff/200001/content/test/data/acce...
File
content/test/data/accessibility/aria/input-text-aria-placeholder-expected-blink.txt
(right):

https://codereview.chromium.org/2927033004/diff/200001/content/test/data/acce...
content/test/data/accessibility/aria/input-text-aria-placeholder-expected-blink.txt:12:
++textField description='aria-description5' name='aria-label5'
placeholder='placeholder5' descriptionFrom=5
At some point later I'd like to pretty-print nameFrom and
descriptionFrom. Do you think it's worth including them in the
meantime?

[commit-bot: I haz the power, 2017-06-13 21:03:06]

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

[commit-bot: I haz the power, 2017-06-13 21:03:07]

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_...)

[aleventhal, 2017-06-14 13:17:26]

https://codereview.chromium.org/2927033004/diff/200001/content/browser/access...
File content/browser/accessibility/dump_accessibility_tree_browsertest.cc
(right):

https://codereview.chromium.org/2927033004/diff/200001/content/browser/access...
content/browser/accessibility/dump_accessibility_tree_browsertest.cc:58: void
AddDefaultFilters(std::vector<Filter>* filters) override {
On 2017/06/13 20:25:33, dmazzoni wrote:
> Since this function is getting long, declare it here and then
> define it below the class. That lets you indent by 2 less,
> might help wrapping a tiny bit but definitely will make the
> class declaration a bit more readable.

Done.

https://codereview.chromium.org/2927033004/diff/200001/content/browser/access...
content/browser/accessibility/dump_accessibility_tree_browsertest.cc:63:
filters->push_back(Filter(base::ASCIIToUTF16("ANIMATED*"), Filter::ALLOW));
On 2017/06/13 20:25:33, dmazzoni wrote:
> Do we actually use this?
> 
> I'm tempted to get rid of ones we don't actually use *at all*,
> or at least group them separately with a comment "not actually
> used yet".

I want to keep them when we don't use them, to make sure we never do
unintentionally.

https://codereview.chromium.org/2927033004/diff/200001/content/browser/access...
content/browser/accessibility/dump_accessibility_tree_browsertest.cc:73: //
filters->push_back(Filter(base::ASCIIToUTF16("HOTTRACKED"),
On 2017/06/13 20:25:33, dmazzoni wrote:
> Maybe put all of the cpmmented-out off by default ones at the top or bottom.
> 
> Feel free to use more whitespace in this function, like:
> 
> //
> // Windows
> //
> 
> // Windows states to log by default
> filters->push_back(
> filters->push_back(
> filters->push_back(
> 
> // Other Windows attributes
> 

Done.

https://codereview.chromium.org/2927033004/diff/200001/content/browser/access...
content/browser/accessibility/dump_accessibility_tree_browsertest.cc:122:
filters->push_back(Filter(base::ASCIIToUTF16("descript*"), Filter::ALLOW));
On 2017/06/13 20:25:33, dmazzoni wrote:
> I vote for either both nameFrom and descriptionFrom, or neither - but having
> descriptionFrom but not nameFrom feels wrong

I wanted to keep descriptionFrom because it's rare, whereas nameFrom is crazy
common.

https://codereview.chromium.org/2927033004/diff/200001/content/browser/access...
content/browser/accessibility/dump_accessibility_tree_browsertest.cc:137:
filters->push_back(Filter(base::ASCIIToUTF16("horizontal"), Filter::ALLOW));
On 2017/06/13 20:25:33, dmazzoni wrote:
> This should probably be an orientation enum {horizontal, veritical,
> not_applicable}, right?
> 
> Does this one show up a lot? I think it might be distracting

Let's make it an enum at some point. For now, horizontal is rare so not very
distracting.

https://codereview.chromium.org/2927033004/diff/200001/content/test/data/acce...
File
content/test/data/accessibility/aria/input-text-aria-placeholder-expected-blink.txt
(right):

https://codereview.chromium.org/2927033004/diff/200001/content/test/data/acce...
content/test/data/accessibility/aria/input-text-aria-placeholder-expected-blink.txt:12:
++textField description='aria-description5' name='aria-label5'
placeholder='placeholder5' descriptionFrom=5
On 2017/06/13 20:25:34, dmazzoni wrote:
> At some point later I'd like to pretty-print nameFrom and
> descriptionFrom. Do you think it's worth including them in the
> meantime?

There are some others to pretty print as well, such as checkedState. Any enum I
guess. Let's include stuff for now and prettify later.

[aleventhal, 2017-06-14 13:17:45]

The CQ bit was checked by aleventhal@chromium.org

[aleventhal, 2017-06-14 13:17:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/2927033004/#ps220001
(title: "Address review comments")

[commit-bot: I haz the power, 2017-06-14 13:17:55]

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, 2017-06-14 14:38:57]

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

[commit-bot: I haz the power, 2017-06-14 14:38:58]

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_...)

[aleventhal, 2017-06-14 14:44:55]

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

[commit-bot: I haz the power, 2017-06-14 14:45:21]

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, 2017-06-14 15:47:37]

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

[commit-bot: I haz the power, 2017-06-14 15:47:38]

Dry run: This issue passed the CQ dry run.

[aleventhal, 2017-06-14 15:50:36]

The CQ bit was checked by aleventhal@chromium.org

[aleventhal, 2017-06-14 15:50:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/2927033004/#ps240001
(title: "Update Windows test results")

[commit-bot: I haz the power, 2017-06-14 15:51:00]

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, 2017-06-14 15:56:03]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1497455436782580,
"parent_rev": "19c5a8a7ff6d04e4688a1a88a3d57ef620fab6ab", "commit_rev":
"92bfea769dea215c29270a96fcd56695c3d8c74e"}

[commit-bot: I haz the power, 2017-06-14 15:56:14]

Description was changed from

==========
Expand test defaults to allow more properties by default in Windows and Blink
tests.

This is a precursor to including more blink test results in general.

BUG=None
==========

to

==========
Expand test defaults to allow more properties by default in Windows and Blink
tests.

This is a precursor to including more blink test results in general.

BUG=None

Review-Url: https://codereview.chromium.org/2927033004
Cr-Commit-Position: refs/heads/master@{#479401}
Committed:
https://chromium.googlesource.com/chromium/src/+/92bfea769dea215c29270a96fcd5...
==========

[commit-bot: I haz the power, 2017-06-14 15:56:16]

Committed patchset #13 (id:240001) as
https://chromium.googlesource.com/chromium/src/+/92bfea769dea215c29270a96fcd5...