Add more fine-grained accessibility modes.

Add more fine-grained accessibility modes.

This is just a refactoring change, there should be no change in behavior yet.

Currently Chrome accessibility support is basically off of on. This change
adds some additional accessibility mode flags so we will be able to selectively
enable just part of accessibility support. See the linked bug for more details.

The idea of casting the accessibility mode bitmask to an enum of all possible
valid accessibility modes doesn't really make sense anymore as the number of
possible valid permutations is now large, so I'm eliminating
AccessibilityModeHelper and making AccessibilityMode into just an integer
type rather than an enum.

Following changes will implement support for these new modes and enable
tracking and manually toggling their usage.

BUG=672205
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/dd3d51a7c8fb4e09d3c3bc0057408a5e1f43899a
Cr-Commit-Position: refs/heads/master@{#438547}

[dmazzoni, 2016-12-07 20:03:57]

Description was changed from

==========
Add more fine-grained accessibility modes.

This is just a refactoring change, there should be no change in behavior yet.

Currently Chrome accessibility support is basically off of on. This change
adds some additional accessibility mode flags so we will be able to selectively
enable just part of accessibility support. See the linked bug for more details.

The idea of casting the accessibility mode bitmask to an enum of all possible
valid accessibility modes doesn't really make sense anymore as the number of
possible valid permutations is now large, so I'm eliminating
AccessibilityModeHelper and making AccessibilityMode into just an integer
type rather than an enum.

Following changes will implement support for these new modes and enable
tracking and manually toggling their usage.

BUG=672205
==========

to

==========
Add more fine-grained accessibility modes.

This is just a refactoring change, there should be no change in behavior yet.

Currently Chrome accessibility support is basically off of on. This change
adds some additional accessibility mode flags so we will be able to selectively
enable just part of accessibility support. See the linked bug for more details.

The idea of casting the accessibility mode bitmask to an enum of all possible
valid accessibility modes doesn't really make sense anymore as the number of
possible valid permutations is now large, so I'm eliminating
AccessibilityModeHelper and making AccessibilityMode into just an integer
type rather than an enum.

Following changes will implement support for these new modes and enable
tracking and manually toggling their usage.

BUG=672205
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[dmazzoni, 2016-12-07 20:24:00]

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

[commit-bot: I haz the power, 2016-12-07 20:24:32]

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

[dmazzoni, 2016-12-07 20:24:46]

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

[dmazzoni, 2016-12-07 20:24:47]

Alice, could you take an initial pass before I get
owners to review?

[commit-bot: I haz the power, 2016-12-07 20:53:12]

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

[commit-bot: I haz the power, 2016-12-07 20:53:13]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[aboxhall, 2016-12-12 20:18:29]

Mostly looks great, just a couple of things I'm confused about.

https://codereview.chromium.org/2558933002/diff/20001/content/browser/accessi...
File content/browser/accessibility/browser_accessibility_state_impl.cc (right):

https://codereview.chromium.org/2558933002/diff/20001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_state_impl.cc:133: mode &=
(AccessibilityModeFlagNativeAPIs |
Can you explain the change here?

https://codereview.chromium.org/2558933002/diff/20001/content/common/accessib...
File content/common/accessibility_mode_enums.h (left):

https://codereview.chromium.org/2558933002/diff/20001/content/common/accessib...
content/common/accessibility_mode_enums.h:18: AccessibilityModeFlagFullTree = 1
<< 1,
Did this comment become inaccurate?

[dmazzoni, 2016-12-12 22:12:16]

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

[commit-bot: I haz the power, 2016-12-12 22:13:24]

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

[dmazzoni, 2016-12-12 22:15:36]

https://codereview.chromium.org/2558933002/diff/20001/content/browser/accessi...
File content/browser/accessibility/browser_accessibility_state_impl.cc (right):

https://codereview.chromium.org/2558933002/diff/20001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_state_impl.cc:133: mode &=
(AccessibilityModeFlagNativeAPIs |
On 2016/12/12 20:18:29, aboxhall wrote:
> Can you explain the change here?

Hmmm, I think what I was trying to do was make the
flag affect some modes but not others, but I think I'll
stick with the change description's promise to make this
just a refactoring change and not affect behavior yet.

Reverted this line back to just return, so if you
run with --disable-renderer-accessibility,
no accessibility mode flags will be enabled.

https://codereview.chromium.org/2558933002/diff/20001/content/common/accessib...
File content/common/accessibility_mode_enums.h (left):

https://codereview.chromium.org/2558933002/diff/20001/content/common/accessib...
content/common/accessibility_mode_enums.h:18: AccessibilityModeFlagFullTree = 1
<< 1,
On 2016/12/12 20:18:29, aboxhall wrote:
> Did this comment become inaccurate?

Yeah, a while back we had an "EditableTextOnly"
mode, and "FullTree" made sense when juxtaposed with
that. But now that we don't have an EditableTextOnly
mode, FullTree doesn't really make sense.

What this mode *actually* does is make the web contents
get an accessibility tree, but doesn't necessarily
expose that tree to content/browser unless the first
flag is set.

In the context of supporting lots of accessibility modes,
this is going to be the minimal mode that has to be enabled
for the web to be accessible, but it won't imply
necessarily exposing ALL accessibility attributes anymore,
just the minimal ones.

[aboxhall, 2016-12-12 22:24:59]

lgtm

[dmazzoni, 2016-12-12 23:49:15]

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

[dmazzoni, 2016-12-12 23:49:16]

+jam for owners review of content/
+nasko for content/common/frame_messages.h

[commit-bot: I haz the power, 2016-12-13 01:29:21]

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

[commit-bot: I haz the power, 2016-12-13 01:29:22]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[jam, 2016-12-13 01:35:12]

https://codereview.chromium.org/2558933002/diff/40001/content/common/accessib...
File content/common/accessibility_mode_enums.h (right):

https://codereview.chromium.org/2558933002/diff/40001/content/common/accessib...
content/common/accessibility_mode_enums.h:16: AccessibilityModeFlagNativeAPIs =
1 << 0,
chromium style guide says this should be ACCESSIBILITY_MODE_FLAG_NATIVE_APIS,
how come it doesn't follow it?
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md

the google style guide (which above says not to follow for consistency) also has
values starting with k. It's confusing to see constants named as this.

[dmazzoni, 2016-12-13 19:41:45]

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

[dmazzoni, 2016-12-13 19:41:51]

https://codereview.chromium.org/2558933002/diff/40001/content/common/accessib...
File content/common/accessibility_mode_enums.h (right):

https://codereview.chromium.org/2558933002/diff/40001/content/common/accessib...
content/common/accessibility_mode_enums.h:16: AccessibilityModeFlagNativeAPIs =
1 << 0,
On 2016/12/13 01:35:12, jam wrote:
> chromium style guide says this should be ACCESSIBILITY_MODE_FLAG_NATIVE_APIS,
> how come it doesn't follow it?
> https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md
> 
> the google style guide (which above says not to follow for consistency) also
has
> values starting with k. It's confusing to see constants named as this.

You're right, not sure why it was like this originally.

I switched all of the enums/constants here to all uppercase.

[commit-bot: I haz the power, 2016-12-13 19:42:27]

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-12-13 21:18:13]

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

[commit-bot: I haz the power, 2016-12-13 21:18:14]

Dry run: This issue passed the CQ dry run.

[jam, 2016-12-14 17:42:04]

lgtm

[dmazzoni, 2016-12-14 17:44:57]

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

[dmazzoni, 2016-12-14 17:44:58]

+tsepez for trivial change to content/common/frame_messages.h

[Tom Sepez, 2016-12-14 18:03:18]

messages LGTM

[dmazzoni, 2016-12-14 18:09:25]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-12-14 18:09:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
aboxhall@chromium.org
Link to the patchset: https://codereview.chromium.org/2558933002/#ps60001
(title: "Reformat enums as uppercase")

[commit-bot: I haz the power, 2016-12-14 18:10:03]

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-12-14 18:41:05]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481738965707480,
"parent_rev": "1bc4c0a32fbd6f0e8d9cce08a43e5c7931d2c458", "commit_rev":
"58a2a559679e301152aee1ba04e69970902acb80"}

[commit-bot: I haz the power, 2016-12-14 18:41:32]

Description was changed from

==========
Add more fine-grained accessibility modes.

This is just a refactoring change, there should be no change in behavior yet.

Currently Chrome accessibility support is basically off of on. This change
adds some additional accessibility mode flags so we will be able to selectively
enable just part of accessibility support. See the linked bug for more details.

The idea of casting the accessibility mode bitmask to an enum of all possible
valid accessibility modes doesn't really make sense anymore as the number of
possible valid permutations is now large, so I'm eliminating
AccessibilityModeHelper and making AccessibilityMode into just an integer
type rather than an enum.

Following changes will implement support for these new modes and enable
tracking and manually toggling their usage.

BUG=672205
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add more fine-grained accessibility modes.

This is just a refactoring change, there should be no change in behavior yet.

Currently Chrome accessibility support is basically off of on. This change
adds some additional accessibility mode flags so we will be able to selectively
enable just part of accessibility support. See the linked bug for more details.

The idea of casting the accessibility mode bitmask to an enum of all possible
valid accessibility modes doesn't really make sense anymore as the number of
possible valid permutations is now large, so I'm eliminating
AccessibilityModeHelper and making AccessibilityMode into just an integer
type rather than an enum.

Following changes will implement support for these new modes and enable
tracking and manually toggling their usage.

BUG=672205
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2558933002
==========

[commit-bot: I haz the power, 2016-12-14 18:41:33]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-14 18:46:01]

Description was changed from

==========
Add more fine-grained accessibility modes.

This is just a refactoring change, there should be no change in behavior yet.

Currently Chrome accessibility support is basically off of on. This change
adds some additional accessibility mode flags so we will be able to selectively
enable just part of accessibility support. See the linked bug for more details.

The idea of casting the accessibility mode bitmask to an enum of all possible
valid accessibility modes doesn't really make sense anymore as the number of
possible valid permutations is now large, so I'm eliminating
AccessibilityModeHelper and making AccessibilityMode into just an integer
type rather than an enum.

Following changes will implement support for these new modes and enable
tracking and manually toggling their usage.

BUG=672205
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2558933002
==========

to

==========
Add more fine-grained accessibility modes.

This is just a refactoring change, there should be no change in behavior yet.

Currently Chrome accessibility support is basically off of on. This change
adds some additional accessibility mode flags so we will be able to selectively
enable just part of accessibility support. See the linked bug for more details.

The idea of casting the accessibility mode bitmask to an enum of all possible
valid accessibility modes doesn't really make sense anymore as the number of
possible valid permutations is now large, so I'm eliminating
AccessibilityModeHelper and making AccessibilityMode into just an integer
type rather than an enum.

Following changes will implement support for these new modes and enable
tracking and manually toggling their usage.

BUG=672205
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/dd3d51a7c8fb4e09d3c3bc0057408a5e1f43899a
Cr-Commit-Position: refs/heads/master@{#438547}
==========

[commit-bot: I haz the power, 2016-12-14 18:46:02]

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