Add support for retrieving image thumbnails as part of the accessibility tree.

Add support for retrieving image thumbnails as part of the accessibility tree.

They're only fetched on demand for one element at a time, and they're
scaled down to a given maximum size and then returned as a data url
for simplicity.

The intended purpose of this feature is for displaying graphics on a
potentially future braille device, but it could be useful for other things
too.

BUG=655273
Committed: https://crrev.com/66181cb4955b94a6e7942fe785e6adad874459e7
Cr-Commit-Position: refs/heads/master@{#434227}

[dmazzoni, 2016-11-22 03:31:14]

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

[commit-bot: I haz the power, 2016-11-22 03:31:40]

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-11-22 03:35:28]

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

[commit-bot: I haz the power, 2016-11-22 03:35:28]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dmazzoni, 2016-11-22 05:20:49]

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

[commit-bot: I haz the power, 2016-11-22 05:21:05]

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-11-22 05:58:05]

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

[commit-bot: I haz the power, 2016-11-22 05:58:06]

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

[dmazzoni, 2016-11-22 17:02:28]

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

[commit-bot: I haz the power, 2016-11-22 17:02:42]

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

[dmazzoni, 2016-11-22 17:03:13]

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

[aboxhall, 2016-11-22 19:06:31]

https://codereview.chromium.org/2522543004/diff/40001/chrome/common/extension...
File chrome/common/extensions/api/automation.idl (right):

https://codereview.chromium.org/2522543004/diff/40001/chrome/common/extension...
chrome/common/extensions/api/automation.idl:580: static void getImageData(long
maxWidth, long maxHeight);
As discussed, please clarify how to return original dimensions.

https://codereview.chromium.org/2522543004/diff/40001/content/renderer/access...
File content/renderer/accessibility/blink_ax_tree_source.h (right):

https://codereview.chromium.org/2522543004/diff/40001/content/renderer/access...
content/renderer/accessibility/blink_ax_tree_source.h:59: // time can be
designated
Unfinished sentence?

https://codereview.chromium.org/2522543004/diff/40001/content/renderer/access...
content/renderer/accessibility/blink_ax_tree_source.h:115: // THe id of the
object to fetch image data for.
Typo "THe"

https://codereview.chromium.org/2522543004/diff/40001/content/renderer/access...
File content/renderer/accessibility/render_accessibility_impl.cc (right):

https://codereview.chromium.org/2522543004/diff/40001/content/renderer/access...
content/renderer/accessibility/render_accessibility_impl.cc:586:
tree_source_.set_image_data_node_id(obj.axID());
Does this get reset to -1 at some point? Or will we continue to send down image
data for this node until we request image data for another one? Or does the tree
source fall out of scope at some point?

https://codereview.chromium.org/2522543004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp (right):

https://codereview.chromium.org/2522543004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp:880: if
(!imageBitmap)
It would be neat at some point if we could render arbitrary elements.

https://codereview.chromium.org/2522543004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp:893: if
(scale >= 1.0)
Does this mean you can't scale the image up? I can't imagine why you'd want to,
but maybe worth documenting.

[commit-bot: I haz the power, 2016-11-22 19:59:13]

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

[commit-bot: I haz the power, 2016-11-22 19:59:14]

Dry run: This issue passed the CQ dry run.

[dmazzoni, 2016-11-22 22:00:11]

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

[dmazzoni, 2016-11-22 22:00:13]

https://codereview.chromium.org/2522543004/diff/40001/chrome/common/extension...
File chrome/common/extensions/api/automation.idl (right):

https://codereview.chromium.org/2522543004/diff/40001/chrome/common/extension...
chrome/common/extensions/api/automation.idl:580: static void getImageData(long
maxWidth, long maxHeight);
On 2016/11/22 19:06:31, aboxhall wrote:
> As discussed, please clarify how to return original dimensions.

Done.

https://codereview.chromium.org/2522543004/diff/40001/content/renderer/access...
File content/renderer/accessibility/blink_ax_tree_source.h (right):

https://codereview.chromium.org/2522543004/diff/40001/content/renderer/access...
content/renderer/accessibility/blink_ax_tree_source.h:59: // time can be
designated
On 2016/11/22 19:06:31, aboxhall wrote:
> Unfinished sentence?

Done.

https://codereview.chromium.org/2522543004/diff/40001/content/renderer/access...
content/renderer/accessibility/blink_ax_tree_source.h:115: // THe id of the
object to fetch image data for.
On 2016/11/22 19:06:31, aboxhall wrote:
> Typo "THe"

Done.

https://codereview.chromium.org/2522543004/diff/40001/content/renderer/access...
File content/renderer/accessibility/render_accessibility_impl.cc (right):

https://codereview.chromium.org/2522543004/diff/40001/content/renderer/access...
content/renderer/accessibility/render_accessibility_impl.cc:586:
tree_source_.set_image_data_node_id(obj.axID());
On 2016/11/22 19:06:31, aboxhall wrote:
> Does this get reset to -1 at some point? Or will we continue to send down
image
> data for this node until we request image data for another one? Or does the
tree
> source fall out of scope at some point?

It doesn't get reset currently, but the client could just call
it again with another node. If it's anything other than an image
it will have no effect, so you could just reset it to the root.

https://codereview.chromium.org/2522543004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp (right):

https://codereview.chromium.org/2522543004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp:880: if
(!imageBitmap)
On 2016/11/22 19:06:31, aboxhall wrote:
> It would be neat at some point if we could render arbitrary elements.

True. Not sure if it'd make sense to render them in isolation, or
snip out part of the webpage, or what.

https://codereview.chromium.org/2522543004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp:893: if
(scale >= 1.0)
On 2016/11/22 19:06:31, aboxhall wrote:
> Does this mean you can't scale the image up? I can't imagine why you'd want
to,
> but maybe worth documenting.

Yes, that's how I've designed it. Doesn't the comment above
cover it? It says that it ensures the output image is no
larger than |maxSize|.

My intent is to set it to the size of a braille display.

If we wanted to display this in dev tools we could set it
to a fixed thumbnail size or something like that.

[commit-bot: I haz the power, 2016-11-22 22:00:57]

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

[aboxhall, 2016-11-22 22:04:51]

lgtm

https://codereview.chromium.org/2522543004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp (right):

https://codereview.chromium.org/2522543004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp:893: if
(scale >= 1.0)
On 2016/11/22 22:00:13, dmazzoni wrote:
> On 2016/11/22 19:06:31, aboxhall wrote:
> > Does this mean you can't scale the image up? I can't imagine why you'd want
> to,
> > but maybe worth documenting.
> 
> Yes, that's how I've designed it. Doesn't the comment above
> cover it? It says that it ensures the output image is no
> larger than |maxSize|.
> 
> My intent is to set it to the size of a braille display.
> 
> If we wanted to display this in dev tools we could set it
> to a fixed thumbnail size or something like that.

I mean you can't pass something larger than the size of the image for |maxSize|
and have it return something larger than the original image. That's not
explicitly mentioned in the comment - it's strictly true that it won't be larger
than |maxSize|, but it also won't be any larger than the original image, and
won't scale.

[dmazzoni, 2016-11-22 22:27:03]

https://codereview.chromium.org/2522543004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp (right):

https://codereview.chromium.org/2522543004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp:893: if
(scale >= 1.0)
On 2016/11/22 22:04:51, aboxhall wrote:
> On 2016/11/22 22:00:13, dmazzoni wrote:
> > On 2016/11/22 19:06:31, aboxhall wrote:
> > > Does this mean you can't scale the image up? I can't imagine why you'd
want
> > to,
> > > but maybe worth documenting.
> > 
> > Yes, that's how I've designed it. Doesn't the comment above
> > cover it? It says that it ensures the output image is no
> > larger than |maxSize|.
> > 
> > My intent is to set it to the size of a braille display.
> > 
> > If we wanted to display this in dev tools we could set it
> > to a fixed thumbnail size or something like that.
> 
> I mean you can't pass something larger than the size of the image for
|maxSize|
> and have it return something larger than the original image. That's not
> explicitly mentioned in the comment - it's strictly true that it won't be
larger
> than |maxSize|, but it also won't be any larger than the original image, and
> won't scale.

Clarified in comment.

[dmazzoni, 2016-11-22 22:27:25]

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

[dmazzoni, 2016-11-22 22:27:26]

+mkwst for owners review

[Mike West, 2016-11-23 13:04:07]

With the caveat that this doesn't seem like something we could reasonably expose
to the web (because it would let you read image data cross-origin), this LGTM.

[dmazzoni, 2016-11-23 17:35:23]

On 2016/11/23 13:04:07, Mike West (slow) wrote:
> With the caveat that this doesn't seem like something we could reasonably
expose
> to the web (because it would let you read image data cross-origin), this LGTM.

Agreed, this is only for exposing to native APIs or system services, not to
the web. That's true of most of accessibility. Thanks.

[dmazzoni, 2016-11-23 17:35:45]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-11-23 17:35:45]

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/2522543004/#ps80001
(title: "Clarified maxSize")

[commit-bot: I haz the power, 2016-11-23 17:36:06]

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-11-23 19:45:02]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1479922545550470,
"parent_rev": "4416ddbcc32f648229262b13a2c5342142e35ffe", "commit_rev":
"b764022678d88bf5673fd54d02db8af669353643"}

[commit-bot: I haz the power, 2016-11-23 19:45:27]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-23 19:48:13]

Description was changed from

==========
Add support for retrieving image thumbnails as part of the accessibility tree.

They're only fetched on demand for one element at a time, and they're
scaled down to a given maximum size and then returned as a data url
for simplicity.

The intended purpose of this feature is for displaying graphics on a
potentially future braille device, but it could be useful for other things
too.

BUG=655273
==========

to

==========
Add support for retrieving image thumbnails as part of the accessibility tree.

They're only fetched on demand for one element at a time, and they're
scaled down to a given maximum size and then returned as a data url
for simplicity.

The intended purpose of this feature is for displaying graphics on a
potentially future braille device, but it could be useful for other things
too.

BUG=655273

Committed: https://crrev.com/66181cb4955b94a6e7942fe785e6adad874459e7
Cr-Commit-Position: refs/heads/master@{#434227}
==========

[commit-bot: I haz the power, 2016-11-23 19:48:14]

Patchset 5 (id:??) landed as
https://crrev.com/66181cb4955b94a6e7942fe785e6adad874459e7
Cr-Commit-Position: refs/heads/master@{#434227}