Adds new experimental accessibility extension api to enable or disable native accessibility.

Makes native and extension-based accessibility mutually exclusive.

BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=198196

[dmazzoni, 2013-04-24 23:08:05]

Something like this could work.

Note that this wouldn't turn off accessibility for all open tabs. We should add
that. We could do it lazily by having RenderWidgetHostView* detect if the
accessibility state gets flipped off. Also, let's hook in the
chrome://accessibility page and make it do the same thing.

I think it'd be better to not tie it to SetAccessibilityEnabled, though, for a
few reasons:

* SetAccessibilityEnabled is intended for views accessibility, i.e.
chrome-of-chrome. It's really independent of accessibility within each tab.
* We might want to use the accessibility extension api for things other than
ChromeVox, like voice control

Another thought: we probably don't want to turn off native accessibility for a
tab that the extension was unable to inject into - due to a lack of priveleges,
or another error. How about we enable or disable native accessibility on a
per-tab basis? ChromeVox could send a message to the background page that would
disable native accessibility for that page only when it's successfully loaded.

This would also make it possible to choose to load ChromeVox only in specific
sites.

[David Tseng, 2013-04-25 16:36:56]

Ok.

How about, for the sake of clarity, I introduced a new method to
experimental.accessibility called setWebAccessibilityEnabled? (or,
setWebAccessibilityMode)?

It would take a tab id, and either a boolean or one of three values that
maps to the AccessibilityMode enum.




On Wed, Apr 24, 2013 at 4:08 PM, <dmazzoni@chromium.org> wrote:

> Something like this could work.
>
> Note that this wouldn't turn off accessibility for all open tabs. We
> should add
> that. We could do it lazily by having RenderWidgetHostView* detect if the
> accessibility state gets flipped off. Also, let's hook in the
> chrome://accessibility page and make it do the same thing.
>
> I think it'd be better to not tie it to SetAccessibilityEnabled, though,
> for a
> few reasons:
>
> * SetAccessibilityEnabled is intended for views accessibility, i.e.
> chrome-of-chrome. It's really independent of accessibility within each tab.
> * We might want to use the accessibility extension api for things other
> than
> ChromeVox, like voice control
>
> Another thought: we probably don't want to turn off native accessibility
> for a
> tab that the extension was unable to inject into - due to a lack of
> priveleges,
> or another error. How about we enable or disable native accessibility on a
> per-tab basis? ChromeVox could send a message to the background page that
> would
> disable native accessibility for that page only when it's successfully
> loaded.
>
> This would also make it possible to choose to load ChromeVox only in
> specific
> sites.
>
>
>
https://codereview.chromium.**org/14473011/<https://codereview.chromium.org/1...
>

[dmazzoni, 2013-04-25 16:54:19]

Sounds good.

No need to expose the "FocusOnly" accessibility mode, but it couldn't hurt
to have an enum in case we have multiple modes in the future. I've been
considering adding a different mode for testing.

- Dominic

On Thu, Apr 25, 2013 at 9:35 AM, David Tseng <dtseng@chromium.org> wrote:

> Ok.
>
> How about, for the sake of clarity, I introduced a new method to
> experimental.accessibility called setWebAccessibilityEnabled? (or,
> setWebAccessibilityMode)?
>
> It would take a tab id, and either a boolean or one of three values that
> maps to the AccessibilityMode enum.
>
>
>
>
> On Wed, Apr 24, 2013 at 4:08 PM, <dmazzoni@chromium.org> wrote:
>
>> Something like this could work.
>>
>> Note that this wouldn't turn off accessibility for all open tabs. We
>> should add
>> that. We could do it lazily by having RenderWidgetHostView* detect if the
>> accessibility state gets flipped off. Also, let's hook in the
>> chrome://accessibility page and make it do the same thing.
>>
>> I think it'd be better to not tie it to SetAccessibilityEnabled, though,
>> for a
>> few reasons:
>>
>> * SetAccessibilityEnabled is intended for views accessibility, i.e.
>> chrome-of-chrome. It's really independent of accessibility within each
>> tab.
>> * We might want to use the accessibility extension api for things other
>> than
>> ChromeVox, like voice control
>>
>> Another thought: we probably don't want to turn off native accessibility
>> for a
>> tab that the extension was unable to inject into - due to a lack of
>> priveleges,
>> or another error. How about we enable or disable native accessibility on a
>> per-tab basis? ChromeVox could send a message to the background page that
>> would
>> disable native accessibility for that page only when it's successfully
>> loaded.
>>
>> This would also make it possible to choose to load ChromeVox only in
>> specific
>> sites.
>>
>>
>>
https://codereview.chromium.**org/14473011/<https://codereview.chromium.org/1...
>>
>
>

[David Tseng, 2013-04-25 19:27:54]

A few more notes. BrowserAccessibilityState appears to turn off
accessibility for all tabs and windows. This actually seems to fit
ChromeVox's model of active vs inactive which by design applies to all tabs
and windows. That's the current behavior I'm seeing with this patch.

Without introducing something into BrowserAccessibilityState, it doesn't
seem like we can easily turn off accessibility on a per tab basis and it
doesn't seem to be what we want for ChromeVox.

So, my current thoughts are to still introduce a new api
setWebAccessibilityEnabled that takes a boolean and sets accessibility mode
to either complete or off.

Thoughts?
David


On Thu, Apr 25, 2013 at 9:54 AM, Dominic Mazzoni <dmazzoni@chromium.org>wrote:

> Sounds good.
>
> No need to expose the "FocusOnly" accessibility mode, but it couldn't hurt
> to have an enum in case we have multiple modes in the future. I've been
> considering adding a different mode for testing.
>
> - Dominic
>
>
> On Thu, Apr 25, 2013 at 9:35 AM, David Tseng <dtseng@chromium.org> wrote:
>
>> Ok.
>>
>> How about, for the sake of clarity, I introduced a new method to
>> experimental.accessibility called setWebAccessibilityEnabled? (or,
>> setWebAccessibilityMode)?
>>
>> It would take a tab id, and either a boolean or one of three values that
>> maps to the AccessibilityMode enum.
>>
>>
>>
>>
>> On Wed, Apr 24, 2013 at 4:08 PM, <dmazzoni@chromium.org> wrote:
>>
>>> Something like this could work.
>>>
>>> Note that this wouldn't turn off accessibility for all open tabs. We
>>> should add
>>> that. We could do it lazily by having RenderWidgetHostView* detect if the
>>> accessibility state gets flipped off. Also, let's hook in the
>>> chrome://accessibility page and make it do the same thing.
>>>
>>> I think it'd be better to not tie it to SetAccessibilityEnabled, though,
>>> for a
>>> few reasons:
>>>
>>> * SetAccessibilityEnabled is intended for views accessibility, i.e.
>>> chrome-of-chrome. It's really independent of accessibility within each
>>> tab.
>>> * We might want to use the accessibility extension api for things other
>>> than
>>> ChromeVox, like voice control
>>>
>>> Another thought: we probably don't want to turn off native accessibility
>>> for a
>>> tab that the extension was unable to inject into - due to a lack of
>>> priveleges,
>>> or another error. How about we enable or disable native accessibility on
>>> a
>>> per-tab basis? ChromeVox could send a message to the background page
>>> that would
>>> disable native accessibility for that page only when it's successfully
>>> loaded.
>>>
>>> This would also make it possible to choose to load ChromeVox only in
>>> specific
>>> sites.
>>>
>>>
>>>
https://codereview.chromium.**org/14473011/<https://codereview.chromium.org/1...
>>>
>>
>>
>

[David Tseng, 2013-04-30 20:12:54]

PTAL. Made some updates to the previous approach (added new API).


On Thu, Apr 25, 2013 at 12:27 PM, David Tseng <dtseng@chromium.org> wrote:

> A few more notes. BrowserAccessibilityState appears to turn off
> accessibility for all tabs and windows. This actually seems to fit
> ChromeVox's model of active vs inactive which by design applies to all tabs
> and windows. That's the current behavior I'm seeing with this patch.
>
> Without introducing something into BrowserAccessibilityState, it doesn't
> seem like we can easily turn off accessibility on a per tab basis and it
> doesn't seem to be what we want for ChromeVox.
>
> So, my current thoughts are to still introduce a new api
> setWebAccessibilityEnabled that takes a boolean and sets accessibility mode
> to either complete or off.
>
> Thoughts?
> David
>
>
> On Thu, Apr 25, 2013 at 9:54 AM, Dominic Mazzoni
<dmazzoni@chromium.org>wrote:
>
>> Sounds good.
>>
>> No need to expose the "FocusOnly" accessibility mode, but it couldn't
>> hurt to have an enum in case we have multiple modes in the future. I've
>> been considering adding a different mode for testing.
>>
>> - Dominic
>>
>>
>> On Thu, Apr 25, 2013 at 9:35 AM, David Tseng <dtseng@chromium.org> wrote:
>>
>>> Ok.
>>>
>>> How about, for the sake of clarity, I introduced a new method to
>>> experimental.accessibility called setWebAccessibilityEnabled? (or,
>>> setWebAccessibilityMode)?
>>>
>>> It would take a tab id, and either a boolean or one of three values that
>>> maps to the AccessibilityMode enum.
>>>
>>>
>>>
>>>
>>> On Wed, Apr 24, 2013 at 4:08 PM, <dmazzoni@chromium.org> wrote:
>>>
>>>> Something like this could work.
>>>>
>>>> Note that this wouldn't turn off accessibility for all open tabs. We
>>>> should add
>>>> that. We could do it lazily by having RenderWidgetHostView* detect if
>>>> the
>>>> accessibility state gets flipped off. Also, let's hook in the
>>>> chrome://accessibility page and make it do the same thing.
>>>>
>>>> I think it'd be better to not tie it to SetAccessibilityEnabled,
>>>> though, for a
>>>> few reasons:
>>>>
>>>> * SetAccessibilityEnabled is intended for views accessibility, i.e.
>>>> chrome-of-chrome. It's really independent of accessibility within each
>>>> tab.
>>>> * We might want to use the accessibility extension api for things other
>>>> than
>>>> ChromeVox, like voice control
>>>>
>>>> Another thought: we probably don't want to turn off native
>>>> accessibility for a
>>>> tab that the extension was unable to inject into - due to a lack of
>>>> priveleges,
>>>> or another error. How about we enable or disable native accessibility
>>>> on a
>>>> per-tab basis? ChromeVox could send a message to the background page
>>>> that would
>>>> disable native accessibility for that page only when it's successfully
>>>> loaded.
>>>>
>>>> This would also make it possible to choose to load ChromeVox only in
>>>> specific
>>>> sites.
>>>>
>>>>
>>>>
https://codereview.chromium.**org/14473011/<https://codereview.chromium.org/1...
>>>>
>>>
>>>
>>
>

[dmazzoni, 2013-04-30 20:31:58]

The name isn't clear to me, I thought "set web accessibility enabled" meant the
same as --force-renderer-accessibility, but it means the opposite.

How about the API is more like "set native accessibility enabled", and the
extension would set it to FALSE if it doesn't want native accessibility to
interfere.

This doesn't update each renderer to turn off accessibility - do you want to do
that? Just updating BrowserAccessibilityState will only affect new tabs opened,
not existing tabs.

https://codereview.chromium.org/14473011/diff/6001/chrome/browser/accessibili...
File chrome/browser/accessibility/accessibility_extension_api.cc (right):

https://codereview.chromium.org/14473011/diff/6001/chrome/browser/accessibili...
chrome/browser/accessibility/accessibility_extension_api.cc:186: if (enabled) {
Shouldn't it call something if you pass false as well?

https://codereview.chromium.org/14473011/diff/6001/chrome/browser/accessibili...
chrome/browser/accessibility/accessibility_extension_api.cc:188:
OnExtensionWebAccessibilityEnabled();
indent 2 more

https://codereview.chromium.org/14473011/diff/6001/chrome/browser/accessibili...
File chrome/browser/accessibility/accessibility_extension_api.h (right):

https://codereview.chromium.org/14473011/diff/6001/chrome/browser/accessibili...
chrome/browser/accessibility/accessibility_extension_api.h:90:
EXPERIMENTAL_ACCESSIBILITY_WEBSETACCESSIBILITYENABLED)
WEBSET -> SETWEB

[David Tseng, 2013-04-30 23:19:05]

On Tue, Apr 30, 2013 at 1:31 PM, <dmazzoni@chromium.org> wrote:

> The name isn't clear to me, I thought "set web accessibility enabled"
> meant the
> same as --force-renderer-**accessibility, but it means the opposite.
>
> How about the API is more like "set native accessibility enabled", and the
> extension would set it to FALSE if it doesn't want native accessibility to
> interfere.
>
> Done.


> This doesn't update each renderer to turn off accessibility - do you want
> to do
> that? Just updating BrowserAccessibilityState will only affect new tabs
> opened,
> not existing tabs.
>
> Done. Placed that logic in BrowserAccessibilityState (but can move it to
the new SetNativeAccessibilityEnabled method instead if that' breaks any
current assumptions).

>
>
> https://codereview.chromium.**org/14473011/diff/6001/chrome/**
>
browser/accessibility/**accessibility_extension_api.cc<https://codereview.chromium.org/14473011/diff/6001/chrome/browser/accessibility/accessibility_extension_api.cc>
> File chrome/browser/accessibility/**accessibility_extension_api.cc
> (right):
>
> https://codereview.chromium.**org/14473011/diff/6001/chrome/**
>
browser/accessibility/**accessibility_extension_api.**cc#newcode186<https://codereview.chromium.org/14473011/diff/6001/chrome/browser/accessibility/accessibility_extension_api.cc#newcode186>
> chrome/browser/accessibility/**accessibility_extension_api.**cc:186: if
> (enabled) {
> Shouldn't it call something if you pass false as well?
> Done.
> https://codereview.chromium.**org/14473011/diff/6001/chrome/**
>
browser/accessibility/**accessibility_extension_api.**cc#newcode188<https://codereview.chromium.org/14473011/diff/6001/chrome/browser/accessibility/accessibility_extension_api.cc#newcode188>
> chrome/browser/accessibility/**accessibility_extension_api.**cc:188:
> OnExtensionWebAccessibilityEna**bled();
> indent 2 more
> Done.
> https://codereview.chromium.**org/14473011/diff/6001/chrome/**
>
browser/accessibility/**accessibility_extension_api.h<https://codereview.chromium.org/14473011/diff/6001/chrome/browser/accessibility/accessibility_extension_api.h>
> File chrome/browser/accessibility/**accessibility_extension_api.h (right):
>
> https://codereview.chromium.**org/14473011/diff/6001/chrome/**
>
browser/accessibility/**accessibility_extension_api.h#**newcode90<https://codereview.chromium.org/14473011/diff/6001/chrome/browser/accessibility/accessibility_extension_api.h#newcode90>
> chrome/browser/accessibility/**accessibility_extension_api.h:**90:
> EXPERIMENTAL_ACCESSIBILITY_**WEBSETACCESSIBILITYENABLED)
> WEBSET -> SETWEB
>
>
https://codereview.chromium.**org/14473011/<https://codereview.chromium.org/1...
>

[dmazzoni, 2013-04-30 23:25:08]

lgtm

https://codereview.chromium.org/14473011/diff/14001/chrome/browser/accessibil...
File chrome/browser/accessibility/accessibility_extension_api.cc (right):

https://codereview.chromium.org/14473011/diff/14001/chrome/browser/accessibil...
chrome/browser/accessibility/accessibility_extension_api.cc:188:
OnAccessibilityEnabledManually();
Call this OnAccessibilityEnabledByExtensionAPI and
OnAccessibilityDisabledByExtensionAPI, I'd like to distinguish between manually
and by extension api in the histograms while we're at it.

[David Tseng, 2013-04-30 23:45:03]

On Tue, Apr 30, 2013 at 4:25 PM, <dmazzoni@chromium.org> wrote:

> lgtm
>
>
>
>
> https://codereview.chromium.**org/14473011/diff/14001/**
>
chrome/browser/accessibility/**accessibility_extension_api.cc<https://codereview.chromium.org/14473011/diff/14001/chrome/browser/accessibility/accessibility_extension_api.cc>
> File chrome/browser/accessibility/**accessibility_extension_api.cc
> (right):
>
> https://codereview.chromium.**org/14473011/diff/14001/**
>
chrome/browser/accessibility/**accessibility_extension_api.**cc#newcode188<https://codereview.chromium.org/14473011/diff/14001/chrome/browser/accessibility/accessibility_extension_api.cc#newcode188>
> chrome/browser/accessibility/**accessibility_extension_api.**cc:188:
> OnAccessibilityEnabledManually**();
> Call this OnAccessibilityEnabledByExtens**ionAPI and
> OnAccessibilityDisabledByExten**sionAPI, I'd like to distinguish between
> manually and by extension api in the histograms while we're at it.
> Done.
>
https://codereview.chromium.**org/14473011/<https://codereview.chromium.org/1...
>

[David Tseng, 2013-04-30 23:50:57]

+ joi@chromium.org for content/public
+ sky@chromium.org for content/browser.

[sky, 2013-05-01 00:51:22]

John, could you look at the changes to /browser_accessibility_state_impl.cc.

https://codereview.chromium.org/14473011/diff/18001/content/browser/accessibi...
File content/browser/accessibility/browser_accessibility_state_impl.cc (right):

https://codereview.chromium.org/14473011/diff/18001/content/browser/accessibi...
content/browser/accessibility/browser_accessibility_state_impl.cc:140:
RenderProcessHost::RenderWidgetHostsIterator rwit(
nit: move into for loop (like you did on 131).

https://codereview.chromium.org/14473011/diff/18001/content/browser/accessibi...
content/browser/accessibility/browser_accessibility_state_impl.cc:143:
RenderWidgetHost* rwh = const_cast<RenderWidgetHost*>(
I'm not sure about the const_cast here.

[jam, 2013-05-01 05:19:16]

https://codereview.chromium.org/14473011/diff/18001/content/browser/accessibi...
File content/browser/accessibility/browser_accessibility_state_impl.cc (right):

https://codereview.chromium.org/14473011/diff/18001/content/browser/accessibi...
content/browser/accessibility/browser_accessibility_state_impl.cc:143:
RenderWidgetHost* rwh = const_cast<RenderWidgetHost*>(
On 2013/05/01 00:51:22, sky wrote:
> I'm not sure about the const_cast here.
seems fine to me. i think it's a mistake that the iterator gives const pointers

[David Tseng, 2013-05-01 18:54:22]

On Tue, Apr 30, 2013 at 5:51 PM, <sky@chromium.org> wrote:
John, could you look at the changes to /browser_accessibility_state_impl.cc.


https://codereview.chromium.org/14473011/diff/18001/content/browser/accessibi...
File content/browser/accessibility/browser_accessibility_state_impl.cc
(right):

https://codereview.chromium.org/14473011/diff/18001/content/browser/accessibi...
content/browser/accessibility/browser_accessibility_state_impl.cc:140:
RenderProcessHost::RenderWidgetHostsIterator rwit(
nit: move into for loop (like you did on 131).
Done.

https://codereview.chromium.org/14473011/diff/18001/content/browser/accessibi...
content/browser/accessibility/browser_accessibility_state_impl.cc:143:
RenderWidgetHost* rwh = const_cast<RenderWidgetHost*>(
I'm not sure about the const_cast here.

https://codereview.chromium.org/14473011/

[David Tseng, 2013-05-01 18:56:55]

+ asargent for chrome/common/extensions/api

[asargent_no_longer_on_chrome, 2013-05-02 03:44:01]

lgtm

[Jói, 2013-05-02 09:52:03]

https://codereview.chromium.org/14473011/diff/24001/content/public/browser/br...
File content/public/browser/browser_accessibility_state.h (right):

https://codereview.chromium.org/14473011/diff/24001/content/public/browser/br...
content/public/browser/browser_accessibility_state.h:24: // Called when
accessibility is enabled manually.
An explanation here (and line 27) of why the comment says "manually" whereas the
method name says "ByExtensionAPI" might be helpful.

[jam, 2013-05-02 15:50:20]

https://codereview.chromium.org/14473011/diff/24001/content/public/browser/br...
File content/public/browser/browser_accessibility_state.h (right):

https://codereview.chromium.org/14473011/diff/24001/content/public/browser/br...
content/public/browser/browser_accessibility_state.h:25: virtual void
OnAccessibilityEnabledByExtensionAPI() = 0;
see http://www.chromium.org/developers/content-module
content doesn't know about extensions, as that's a concept from a higher level
module (chrome).

please don't mention extensions in these api. instead make the name something
like EnableAccessibility/DisableAccessibility.

nit: in general, "On" as a prefix is used for IPC message handlers.

[dmazzoni, 2013-05-02 16:00:03]

On 2013/05/02 15:50:20, jam wrote:
> please don't mention extensions in these api. instead make the name something
> like EnableAccessibility/DisableAccessibility.

The purpose for the different methods is for histograms, so we can track the
different reasons why accessibility codepaths are being enabled.

How about EnableAccessibilityManually and EnableAccessibilityFromAPI?
EnableAccessibilityManually would include the command-line flag and the button
in chrome://accessibility, which are explicit user actions, and
EnableAccessibilityFromAPI would capture the extension API without mentioning
extensions specifically.

Alternatively, BrowserAccessibilityState could collapse all of those to a single
EnableAccessibility, and the callers could update the histograms - but I think
it's cleaner to have all of the histograms in one place.

[David Tseng, 2013-05-02 18:35:28]

On Thu, May 2, 2013 at 2:52 AM, <joi@chromium.org> wrote:

https://codereview.chromium.org/14473011/diff/24001/content/public/browser/br...
File content/public/browser/browser_accessibility_state.h (right):

https://codereview.chromium.org/14473011/diff/24001/content/public/browser/br...
content/public/browser/browser_accessibility_state.h:24: // Called when
accessibility is enabled manually.
An explanation here (and line 27) of why the comment says "manually"
whereas the method name says "ByExtensionAPI" might be helpful.

Revised comment.
https://codereview.chromium.org/14473011/

[Jói, 2013-05-02 18:58:44]

LGTM for content/public.

[David Tseng, 2013-05-02 19:03:17]

On Thu, May 2, 2013 at 9:00 AM, <dmazzoni@chromium.org> wrote:
On 2013/05/02 15:50:20, jam wrote:
please don't mention extensions in these api. instead make the name
something
like EnableAccessibility/DisableAccessibility.

The purpose for the different methods is for histograms, so we can track the
different reasons why accessibility codepaths are being enabled.

How about EnableAccessibilityManually and EnableAccessibilityFromAPI?
EnableAccessibilityManually would include the command-line flag and the
button
in chrome://accessibility, which are explicit user actions, and
EnableAccessibilityFromAPI would capture the extension API without
mentioning
extensions specifically.

Alternatively, BrowserAccessibilityState could collapse all of those to a
single
EnableAccessibility, and the callers could update the histograms - but I
think
it's cleaner to have all of the histograms in one place.

I've gone with the latter in this CL since it's the extension API that's
the lone consumer of these two methods and it makes the wording a little
less awkward..
 it looks like accessibility_ui currently just calls SetAccessibilityMode
 directly. We should clean that up in a subsequent cl. Perhaps make most
callers use BrowserAccessibilityState to set accessibility mode at a higher
layer.
Also,

https://codereview.chromium.org/14473011/

[David Tseng, 2013-05-02 20:40:33]

Scott, PTAL?


On Thu, May 2, 2013 at 12:03 PM, David Tseng <dtseng@chromium.org> wrote:

>
>
>
>
> On Thu, May 2, 2013 at 9:00 AM, <dmazzoni@chromium.org> wrote:
> On 2013/05/02 15:50:20, jam wrote:
> please don't mention extensions in these api. instead make the name
> something
> like EnableAccessibility/DisableAccessibility.
>
> The purpose for the different methods is for histograms, so we can track
> the
> different reasons why accessibility codepaths are being enabled.
>
> How about EnableAccessibilityManually and EnableAccessibilityFromAPI?
> EnableAccessibilityManually would include the command-line flag and the
> button
> in chrome://accessibility, which are explicit user actions, and
> EnableAccessibilityFromAPI would capture the extension API without
> mentioning
> extensions specifically.
>
> Alternatively, BrowserAccessibilityState could collapse all of those to a
> single
> EnableAccessibility, and the callers could update the histograms - but I
> think
> it's cleaner to have all of the histograms in one place.
>
> I've gone with the latter in this CL since it's the extension API that's
> the lone consumer of these two methods and it makes the wording a little
> less awkward..
>  it looks like accessibility_ui currently just calls SetAccessibilityMode
>  directly. We should clean that up in a subsequent cl. Perhaps make most
> callers use BrowserAccessibilityState to set accessibility mode at a higher
> layer.
> Also,
>
> https://codereview.chromium.org/14473011/
>
>

[jam, 2013-05-02 22:34:06]

lgtm

https://codereview.chromium.org/14473011/diff/33011/content/public/browser/br...
File content/public/browser/browser_accessibility_state.h (right):

https://codereview.chromium.org/14473011/diff/33011/content/public/browser/br...
content/public/browser/browser_accessibility_state.h:9: 
nit: no blank line

https://codereview.chromium.org/14473011/diff/33011/content/public/browser/br...
content/public/browser/browser_accessibility_state.h:24: // Enables
accessibility for all running tabs.
nit: don't mention tabs in content, since that's a chrome concept. you can say
"WebContents" instead.

[sky, 2013-05-02 22:57:32]

LGTM

[commit-bot: I haz the power, 2013-05-02 23:18:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dtseng@chromium.org/14473011/33011

[commit-bot: I haz the power, 2013-05-02 23:18:05]

Failed to apply patch for
chrome/browser/extensions/extension_function_histogram_value.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/extensions/extension_function_histogram_value.h
  Hunk #1 FAILED at 520.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/extensions/extension_function_histogram_value.h.rej

Patch:       chrome/browser/extensions/extension_function_histogram_value.h
Index: chrome/browser/extensions/extension_function_histogram_value.h
diff --git a/chrome/browser/extensions/extension_function_histogram_value.h
b/chrome/browser/extensions/extension_function_histogram_value.h
index
e73038451339920fc205e2f2e378c293e77c608b..e282f53c6e0ab6c63a4be54c0e2e5c32b402d01b
100644
--- a/chrome/browser/extensions/extension_function_histogram_value.h
+++ b/chrome/browser/extensions/extension_function_histogram_value.h
@@ -520,6 +520,7 @@ enum HistogramValue {
   AUDIO_SETACTIVEDEVICES,
   AUDIO_SETPROPERTIES,
   USB_RESETDEVICE,
+  EXPERIMENTAL_ACCESSIBILITY_SETNATIVEACCESSIBILITYENABLED,
   ENUM_BOUNDARY // Last entry: Add new entries above.
 };

[commit-bot: I haz the power, 2013-05-03 17:05:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dtseng@chromium.org/14473011/43001

[commit-bot: I haz the power, 2013-05-03 21:45:04]

Change committed as 198196