Beginning of an accessibility implementation for html_viewer.

Beginning of an accessibility implementation for html_viewer.

This depends on the interface definition here:
https://codereview.chromium.org/715463002/

Committed: https://crrev.com/34cf1b98e64108d50969638dc39a1b1bc92ee8f7
Cr-Commit-Position: refs/heads/master@{#305080}

[Aaron Boodman, 2014-11-08 07:23:22]

aa@chromium.org changed reviewers:
+ abarth@chromium.org

[Aaron Boodman, 2014-11-08 07:23:43]

I still have to figure out how to hook the test up to the bots, will do that
next week.

[abarth-chromium, 2014-11-10 00:33:05]

This is really wonderful.  LGTM.

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
File mojo/services/html_viewer/ax_provider_impl.cc (right):

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
mojo/services/html_viewer/ax_provider_impl.cc:32: };
Should this go in a more general location for a converter?  It's not specific to
AX.  Maybe
https://code.google.com/p/chromium/codesearch#chromium/src/mojo/services/html...
?

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
File mojo/services/html_viewer/ax_provider_impl.h (right):

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
mojo/services/html_viewer/ax_provider_impl.h:21: AxProviderImpl(blink::WebView*
web_view);
explicit

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
File mojo/services/html_viewer/ax_provider_impl_unittest.cc (right):

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
mojo/services/html_viewer/ax_provider_impl_unittest.cc:54: struct NodeCatcher {
Cute

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
mojo/services/html_viewer/ax_provider_impl_unittest.cc:97:
base::MessageLoop::current()->Run();
I'd probably move this to just below loadHTMLString to clarify that it's related
to that and not to NodeCatcher..

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
mojo/services/html_viewer/ax_provider_impl_unittest.cc:103: auto& node =
catcher.nodes[i];
We can't use for (auto& node : catcher.nodes) ?  Maybe we should improve
mojo::Array to support that.  (Not necessary for this CL.)

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
mojo/services/html_viewer/ax_provider_impl_unittest.cc:115: auto is_baz =
[&is_text](MapEntry pair) { return is_text(pair, "baz"); };
Mind blown!

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
mojo/services/html_viewer/ax_provider_impl_unittest.cc:115: auto is_baz =
[&is_text](MapEntry pair) { return is_text(pair, "baz"); };
Mind blown!

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
File mojo/services/html_viewer/html_document_view.h (right):

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
mojo/services/html_viewer/html_document_view.h:134: std::set<AxProviderImpl*>
ax_provider_impls_;
There's no set equivalent for ScopedVector in base?  It's sad we can't use C++11
collections yet.

[Aaron Boodman, 2014-11-10 07:05:53]

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
File mojo/services/html_viewer/ax_provider_impl.cc (right):

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
mojo/services/html_viewer/ax_provider_impl.cc:32: };
On 2014/11/10 00:33:04, abarth wrote:
> Should this go in a more general location for a converter?  It's not specific
to
> AX.  Maybe
>
https://code.google.com/p/chromium/codesearch#chromium/src/mojo/services/html...
> ?

Whoops, did not see that there. Done.

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
File mojo/services/html_viewer/ax_provider_impl.h (right):

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
mojo/services/html_viewer/ax_provider_impl.h:21: AxProviderImpl(blink::WebView*
web_view);
On 2014/11/10 00:33:04, abarth wrote:
> explicit

Done.

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
File mojo/services/html_viewer/ax_provider_impl_unittest.cc (right):

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
mojo/services/html_viewer/ax_provider_impl_unittest.cc:97:
base::MessageLoop::current()->Run();
On 2014/11/10 00:33:04, abarth wrote:
> I'd probably move this to just below loadHTMLString to clarify that it's
related
> to that and not to NodeCatcher..

Done.

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
mojo/services/html_viewer/ax_provider_impl_unittest.cc:103: auto& node =
catcher.nodes[i];
On 2014/11/10 00:33:04, abarth wrote:
> We can't use for (auto& node : catcher.nodes) ?  Maybe we should improve
> mojo::Array to support that.  (Not necessary for this CL.)

agree.

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
mojo/services/html_viewer/ax_provider_impl_unittest.cc:115: auto is_baz =
[&is_text](MapEntry pair) { return is_text(pair, "baz"); };
On 2014/11/10 00:33:04, abarth wrote:
> Mind blown!

Too bad we can't use stl 11. Could do this with std::bind.

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
File mojo/services/html_viewer/html_document_view.h (right):

https://codereview.chromium.org/710803002/diff/20001/mojo/services/html_viewe...
mojo/services/html_viewer/html_document_view.h:134: std::set<AxProviderImpl*>
ax_provider_impls_;
On 2014/11/10 00:33:04, abarth wrote:
> There's no set equivalent for ScopedVector in base? 

nope.

> It's sad we can't use C++11 collections yet.

yup. I could use linked_ptr, but there's some debate about whether that's a good
idea. STLDeleteElements() is less contentious.

[abarth-chromium, 2014-11-11 05:50:45]

lgtm

[Aaron Boodman, 2014-11-20 20:16:40]

The CQ bit was checked by aa@chromium.org

[commit-bot: I haz the power, 2014-11-20 20:17:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/710803002/100001

[commit-bot: I haz the power, 2014-11-20 20:42:10]

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

[commit-bot: I haz the power, 2014-11-20 20:42:11]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Aaron Boodman, 2014-11-20 20:45:53]

The CQ bit was checked by aa@chromium.org

[commit-bot: I haz the power, 2014-11-20 20:46:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/710803002/120001

[commit-bot: I haz the power, 2014-11-20 21:26:11]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2014-11-20 21:27:08]

Patchset 7 (id:??) landed as
https://crrev.com/34cf1b98e64108d50969638dc39a1b1bc92ee8f7
Cr-Commit-Position: refs/heads/master@{#305080}