Implemented the reporting of text style and language information on Windows.

Implemented the reporting of text style and language information on Windows.

BUG=484228
TESTED=unit test, dump accessibility tree tests, manually with Jaws and NVDA screen readers
R=dmazzoni@chromium.org,aboxhall@chromium.org

Committed: https://crrev.com/0eebf0cd114b70337c0ae3562786e538bd6aac9d
Cr-Commit-Position: refs/heads/master@{#381560}

[dmazzoni, 2016-03-07 23:38:11]

Add DumpAccessibilityTree tests for the style text attributes

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

https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
content/browser/accessibility/browser_accessibility.cc:914: // TODO(nektar):
Figure out if there is an existing Skia function to use
You can use SkColorGetR, etc. from SkColor.h

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

https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_win.cc:2378:
ComputeTextAttributes();
We shouldn't compute them from scratch every time this is called.
If necessary, compute them the first time but then save it so
it doesn't need to be recomputed if the node doesn't change and
this is called again?

https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_win.cc:4010: for (size_t i =
0; i < PlatformChildCount(); ++i) {
Why does this loop over children, but not recursively? It seems like if you have
a static text node, this isn't going to do the right thing, but also if you have
two nested spans like <b><i></i></b> it's not going to work for those either.

I think it has to be recursive.

https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_win.cc:4015: if
(child->ia_role() == ui::AX_ROLE_LIST_MARKER)
The LHS is an IAccessible role but the RHS is a Chrome role

Either way, add a TODO, it'd probably be better if this was computed in Blink
because there are lots of other auto-generated things

[blink-reviews, 2016-03-12 02:27:17]

Add DumpAccessibilityTree tests for the style text attributes
>
> Done. But I also want to ad a browser test in code, to test 
> BrowserAccessibilityWin::GetStartOfStyle()

>
>
https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
> File content/browser/accessibility/browser_accessibility.cc (right):
>
>
https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
> content/browser/accessibility/browser_accessibility.cc:914: //
> TODO(nektar): Figure out if there is an existing Skia function to use
> You can use SkColorGetR, etc. from SkColor.h
>
Done.

>
https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
> File content/browser/accessibility/browser_accessibility_win.cc (right):
>
>
https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
> content/browser/accessibility/browser_accessibility_win.cc:2378:
> ComputeTextAttributes();
> We shouldn't compute them from scratch every time this is called.
> If necessary, compute them the first time but then save it so
> it doesn't need to be recomputed if the node doesn't change and
> this is called again?
>
Yes, that was my intention but I forgot. (Smile.)
Done.

>
https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
> content/browser/accessibility/browser_accessibility_win.cc:4010: for
> (size_t i = 0; i < PlatformChildCount(); ++i) {
> Why does this loop over children, but not recursively? It seems like if
> you have a static text node, this isn't going to do the right thing, but
> also if you have two nested spans like <b><i></i></b> it's not going to
> work for those either.
>
> I think it has to be recursive.
>
You are right about the static-text node. Done.
But being recursive, I am not sure, because get_attributes should only 
return text attributes for valid IA2_hypertext offsets, including 
offsets that point to embedded objects. So, it should return the text 
attributes that are defined on the actual embedded object directly, but 
not on the text leaves of the tree?
For example, if there is a paragraph whose first node is a span with 
style=bold and we ask for the text attributes on the document body at 
the offset where the paragraph is, I don't think that bold should be 
returned because it's not defined on the whole paragraph. Recursively 
going through the children would give us bold.


https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
> content/browser/accessibility/browser_accessibility_win.cc:4015: if
> (child->ia_role() == ui::AX_ROLE_LIST_MARKER)
> The LHS is an IAccessible role but the RHS is a Chrome role
>
Done.

> Either way, add a TODO, it'd probably be better if this was computed in
> Blink because there are lots of other auto-generated things
>
Done.


-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-03-12 02:27:18]

Add DumpAccessibilityTree tests for the style text attributes
>
> Done. But I also want to ad a browser test in code, to test 
> BrowserAccessibilityWin::GetStartOfStyle()

>
>
https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
> File content/browser/accessibility/browser_accessibility.cc (right):
>
>
https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
> content/browser/accessibility/browser_accessibility.cc:914: //
> TODO(nektar): Figure out if there is an existing Skia function to use
> You can use SkColorGetR, etc. from SkColor.h
>
Done.

>
https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
> File content/browser/accessibility/browser_accessibility_win.cc (right):
>
>
https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
> content/browser/accessibility/browser_accessibility_win.cc:2378:
> ComputeTextAttributes();
> We shouldn't compute them from scratch every time this is called.
> If necessary, compute them the first time but then save it so
> it doesn't need to be recomputed if the node doesn't change and
> this is called again?
>
Yes, that was my intention but I forgot. (Smile.)
Done.

>
https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
> content/browser/accessibility/browser_accessibility_win.cc:4010: for
> (size_t i = 0; i < PlatformChildCount(); ++i) {
> Why does this loop over children, but not recursively? It seems like if
> you have a static text node, this isn't going to do the right thing, but
> also if you have two nested spans like <b><i></i></b> it's not going to
> work for those either.
>
> I think it has to be recursive.
>
You are right about the static-text node. Done.
But being recursive, I am not sure, because get_attributes should only 
return text attributes for valid IA2_hypertext offsets, including 
offsets that point to embedded objects. So, it should return the text 
attributes that are defined on the actual embedded object directly, but 
not on the text leaves of the tree?
For example, if there is a paragraph whose first node is a span with 
style=bold and we ask for the text attributes on the document body at 
the offset where the paragraph is, I don't think that bold should be 
returned because it's not defined on the whole paragraph. Recursively 
going through the children would give us bold.


https://codereview.chromium.org/1768753003/diff/20001/content/browser/accessi...
> content/browser/accessibility/browser_accessibility_win.cc:4015: if
> (child->ia_role() == ui::AX_ROLE_LIST_MARKER)
> The LHS is an IAccessible role but the RHS is a Chrome role
>
Done.

> Either way, add a TODO, it'd probably be better if this was computed in
> Blink because there are lots of other auto-generated things
>
Done.


-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmazzoni, 2016-03-14 17:38:21]

This looks really close, I just want to see the expected results for the new CSS
tests

https://codereview.chromium.org/1768753003/diff/40001/content/browser/accessi...
File content/browser/accessibility/browser_accessibility_win.cc (right):

https://codereview.chromium.org/1768753003/diff/40001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_win.cc:2356:
MayBeComputeStyles();
nit: no capital B in maybe, just spell it MaybeComputeStyles

I'd also like ComputeStylesIfNeeded

https://codereview.chromium.org/1768753003/diff/40001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_win.cc:4165:
win_attributes_->offset_to_text_attributes = attributes_map;
This is a deep copy. Instead you could either make attribute_map a reference up
at the top:

auto& attributes_map = win_attributes_->offset_to_text_attributes;

Alternatively, you could swap it:
win_attributes_->offset_to_text_attributes.swap(attributes_map);

https://codereview.chromium.org/1768753003/diff/40001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_win.cc:4261:
offset_to_text_attributes().upper_bound(start_offset);
Just checking, is it correct that both BACKWARDS and FORWARDS call upper_bound?
Shouldn't one call lower_bound to be symmetrical?

[blink-reviews, 2016-03-14 22:49:09]

This looks really close, I just want to see the expected results for the 
new CSS
> tests

> Expectations for Windows added.
>
>
>
https://codereview.chromium.org/1768753003/diff/40001/content/browser/accessi...
> File content/browser/accessibility/browser_accessibility_win.cc (right):
>
>
https://codereview.chromium.org/1768753003/diff/40001/content/browser/accessi...
> content/browser/accessibility/browser_accessibility_win.cc:2356:
> MayBeComputeStyles();
> nit: no capital B in maybe, just spell it MaybeComputeStyles
>
> I'd also like ComputeStylesIfNeeded
>
Done. Second idea.

>
https://codereview.chromium.org/1768753003/diff/40001/content/browser/accessi...
> content/browser/accessibility/browser_accessibility_win.cc:4165:
> win_attributes_->offset_to_text_attributes = attributes_map;
> This is a deep copy. Instead you could either make attribute_map a
> reference up at the top:
>
> auto& attributes_map = win_attributes_->offset_to_text_attributes;
>
> Alternatively, you could swap it:
> win_attributes_->offset_to_text_attributes.swap(attributes_map);
>
Done with swap.

>
https://codereview.chromium.org/1768753003/diff/40001/content/browser/accessi...
> content/browser/accessibility/browser_accessibility_win.cc:4261:
> offset_to_text_attributes().upper_bound(start_offset);
> Just checking, is it correct that both BACKWARDS and FORWARDS call
> upper_bound? Shouldn't one call lower_bound to be symmetrical?

No because the start boundary is inclusive. So if I ask for offset at 5, 
and bold text starts at 5, I should still return 5, not the start offset 
of the previous style run. I.e. I have to find the upper bound, meaning 
the next start offset, and get the previous start offset from that.
However, I think there is a bug there still. I need to write a browser 
test that will interactively call this function.

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-03-14 22:49:09]

This looks really close, I just want to see the expected results for the 
new CSS
> tests

> Expectations for Windows added.
>
>
>
https://codereview.chromium.org/1768753003/diff/40001/content/browser/accessi...
> File content/browser/accessibility/browser_accessibility_win.cc (right):
>
>
https://codereview.chromium.org/1768753003/diff/40001/content/browser/accessi...
> content/browser/accessibility/browser_accessibility_win.cc:2356:
> MayBeComputeStyles();
> nit: no capital B in maybe, just spell it MaybeComputeStyles
>
> I'd also like ComputeStylesIfNeeded
>
Done. Second idea.

>
https://codereview.chromium.org/1768753003/diff/40001/content/browser/accessi...
> content/browser/accessibility/browser_accessibility_win.cc:4165:
> win_attributes_->offset_to_text_attributes = attributes_map;
> This is a deep copy. Instead you could either make attribute_map a
> reference up at the top:
>
> auto& attributes_map = win_attributes_->offset_to_text_attributes;
>
> Alternatively, you could swap it:
> win_attributes_->offset_to_text_attributes.swap(attributes_map);
>
Done with swap.

>
https://codereview.chromium.org/1768753003/diff/40001/content/browser/accessi...
> content/browser/accessibility/browser_accessibility_win.cc:4261:
> offset_to_text_attributes().upper_bound(start_offset);
> Just checking, is it correct that both BACKWARDS and FORWARDS call
> upper_bound? Shouldn't one call lower_bound to be symmetrical?

No because the start boundary is inclusive. So if I ask for offset at 5, 
and bold text starts at 5, I should still return 5, not the start offset 
of the previous style run. I.e. I have to find the upper bound, meaning 
the next start offset, and get the previous start offset from that.
However, I think there is a bug there still. I need to write a browser 
test that will interactively call this function.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[blink-reviews, 2016-03-14 23:37:12]

Okay, color fixed.
I will fix spelling and test style spans (upper-bound) in another patch.

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-03-14 23:37:13]

Okay, color fixed.
I will fix spelling and test style spans (upper-bound) in another patch.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmazzoni, 2016-03-15 04:15:02]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-03-15 04:15:03]

lgtm

[commit-bot: I haz the power, 2016-03-15 04:15:16]

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

[commit-bot: I haz the power, 2016-03-15 04:17:21]

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

[commit-bot: I haz the power, 2016-03-15 04:17:22]

Try jobs failed on following builders:
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[nektarios, 2016-03-15 14:21:17]

The CQ bit was checked by nektar@chromium.org

[commit-bot: I haz the power, 2016-03-15 14:21:26]

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

[commit-bot: I haz the power, 2016-03-15 14:23:21]

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

[commit-bot: I haz the power, 2016-03-15 14:23:22]

Try jobs failed on following builders:
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[nektarios, 2016-03-15 15:24:13]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-03-15 15:24:14]

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/1768753003/#ps120001
(title: "Rebased with master.")

[commit-bot: I haz the power, 2016-03-15 15:24:28]

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

[commit-bot: I haz the power, 2016-03-15 15:31:58]

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

[commit-bot: I haz the power, 2016-03-15 15:32:00]

Try jobs failed on following builders:
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[nektarios, 2016-03-15 15:52:08]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-03-15 15:52:09]

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/1768753003/#ps140001
(title: "Fixed compilation error.")

[commit-bot: I haz the power, 2016-03-15 15:52:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1768753003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1768753003/140001

[commit-bot: I haz the power, 2016-03-15 16:06:04]

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

[commit-bot: I haz the power, 2016-03-15 16:06:06]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[nektarios, 2016-03-15 16:23:39]

Description was changed from

==========
Implemented the reporting of text style and language information on Windows.

BUG=
R=dmazzoni@chromium.org,aboxhall@chromium.org
==========

to

==========
Implemented the reporting of text style and language information on Windows.

BUG=
TESTED=unit test, dump accessibility tree tests, manually with Jaws and NVDA
screen readers
R=dmazzoni@chromium.org,aboxhall@chromium.org
==========

[nektarios, 2016-03-15 16:23:39]

nektar@chromium.org changed reviewers:
+ dglazkov@chromium.org

[nektarios, 2016-03-15 16:24:15]

@dglazkov
Please review public/web/...

[dglazkov, 2016-03-15 17:44:24]

lgtm

[nektarios, 2016-03-15 18:20:12]

Description was changed from

==========
Implemented the reporting of text style and language information on Windows.

BUG=
TESTED=unit test, dump accessibility tree tests, manually with Jaws and NVDA
screen readers
R=dmazzoni@chromium.org,aboxhall@chromium.org
==========

to

==========
Implemented the reporting of text style and language information on Windows.

BUG=484228
TESTED=unit test, dump accessibility tree tests, manually with Jaws and NVDA
screen readers
R=dmazzoni@chromium.org,aboxhall@chromium.org
==========

[nektarios, 2016-03-15 18:20:20]

The CQ bit was checked by nektar@chromium.org

[commit-bot: I haz the power, 2016-03-15 18:20:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1768753003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1768753003/140001

[commit-bot: I haz the power, 2016-03-15 19:10:40]

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

[commit-bot: I haz the power, 2016-03-15 19:10:42]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[nektarios, 2016-03-15 19:16:19]

The CQ bit was checked by nektar@chromium.org

[commit-bot: I haz the power, 2016-03-15 19:17:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1768753003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1768753003/140001

[commit-bot: I haz the power, 2016-03-15 19:35:20]

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

[commit-bot: I haz the power, 2016-03-15 19:35:22]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[nektarios, 2016-03-16 06:48:36]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-03-16 06:48:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org, dglazkov@chromium.org
Link to the patchset: https://codereview.chromium.org/1768753003/#ps160001
(title: "Changed to using an ordered map for the text attriutes.")

[commit-bot: I haz the power, 2016-03-16 06:48:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1768753003/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1768753003/160001

[commit-bot: I haz the power, 2016-03-16 07:32:05]

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

[commit-bot: I haz the power, 2016-03-16 07:32:06]

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

[nektarios, 2016-03-16 15:11:57]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-03-16 15:11:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org, dglazkov@chromium.org
Link to the patchset: https://codereview.chromium.org/1768753003/#ps140002
(title: "Fixed compilation error.")

[commit-bot: I haz the power, 2016-03-16 15:12:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1768753003/140002
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1768753003/140002

[commit-bot: I haz the power, 2016-03-16 16:15:57]

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

[commit-bot: I haz the power, 2016-03-16 16:15:59]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[nektarios, 2016-03-16 18:26:06]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-03-16 18:26:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org, dglazkov@chromium.org
Link to the patchset: https://codereview.chromium.org/1768753003/#ps190001
(title: "Fixed test expectations.")

[commit-bot: I haz the power, 2016-03-16 18:26:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1768753003/190001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1768753003/190001

[commit-bot: I haz the power, 2016-03-16 20:29:07]

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

[commit-bot: I haz the power, 2016-03-16 20:29:09]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[nektarios, 2016-03-16 20:42:44]

The CQ bit was checked by nektar@chromium.org

[commit-bot: I haz the power, 2016-03-16 20:43:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1768753003/190001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1768753003/190001

[commit-bot: I haz the power, 2016-03-16 22:09:02]

Committed patchset #11 (id:190001)

[commit-bot: I haz the power, 2016-03-16 22:12:26]

Description was changed from

==========
Implemented the reporting of text style and language information on Windows.

BUG=484228
TESTED=unit test, dump accessibility tree tests, manually with Jaws and NVDA
screen readers
R=dmazzoni@chromium.org,aboxhall@chromium.org
==========

to

==========
Implemented the reporting of text style and language information on Windows.

BUG=484228
TESTED=unit test, dump accessibility tree tests, manually with Jaws and NVDA
screen readers
R=dmazzoni@chromium.org,aboxhall@chromium.org

Committed: https://crrev.com/0eebf0cd114b70337c0ae3562786e538bd6aac9d
Cr-Commit-Position: refs/heads/master@{#381560}
==========

[commit-bot: I haz the power, 2016-03-16 22:12:28]

Patchset 11 (id:??) landed as
https://crrev.com/0eebf0cd114b70337c0ae3562786e538bd6aac9d
Cr-Commit-Position: refs/heads/master@{#381560}