Accessible bounds should include device scale factor on Windows.

Accessible bounds should include device scale factor on Windows.

When reporting the bounding box of an object from the web
via an accessibility API on Windows, we need to take the device
scale factor into account. Other platforms either don't need this
(Mac) or already take it into account elsewhere (Android).

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

Review-Url: https://codereview.chromium.org/2759903002
Cr-Commit-Position: refs/heads/master@{#458120}
Committed: https://chromium.googlesource.com/chromium/src/+/b1d3174a5e465867b3ae541ba545a47e11ace4b2

[dmazzoni, 2017-03-20 04:29:56]

Description was changed from

==========
Accessible bounds should include device scale factor on Windows.

When reporting the bounding box of an object from the web
via an accessibility API on Windows, we need to take the device
scale factor into account. Other platforms either don't need this
(Mac) or already take it into account elsewhere (Android).

BUG=694605
==========

to

==========
Accessible bounds should include device scale factor on Windows.

When reporting the bounding box of an object from the web
via an accessibility API on Windows, we need to take the device
scale factor into account. Other platforms either don't need this
(Mac) or already take it into account elsewhere (Android).

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

[dmazzoni, 2017-03-20 04:30:03]

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

[commit-bot: I haz the power, 2017-03-20 04:30:10]

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, 2017-03-20 05:26:01]

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

[commit-bot: I haz the power, 2017-03-20 05:26:02]

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

[dmazzoni, 2017-03-20 15:14:24]

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

[commit-bot: I haz the power, 2017-03-20 15:14:51]

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

[dmazzoni, 2017-03-20 15:22:46]

dmazzoni@chromium.org changed reviewers:
+ avi@chromium.org, nektar@chromium.org

[dmazzoni, 2017-03-20 15:23:19]

+avi for content/browser/frame_host owners review

[Avi (use Gerrit), 2017-03-20 15:31:25]

lgtm

[nektarios, 2017-03-20 15:55:10]

1.
+      override_device_scale_factor_for_testing_(false) {}
I thought that override is for actions and overwrite is for values. So,
shouldn't it be
+      overwrite_device_scale_factor_for_testing_(false) {}
See also other related places in the patch.

2. Is there an event that fires when the scale factor changes. Perhaps we can
watch for that? Otherwise it's fine as is.
3. In BrowserAccessibilityWin I left a TODO
// TODO(nektar): Figure out the current value of pixels per inch.
Could you somehow deal with this TODO as part of this patch? I.e. is there a way
to tell the pixels per inch from the scale factor?

[nektarios, 2017-03-20 15:55:25]

lgtm

[commit-bot: I haz the power, 2017-03-20 16:01:15]

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

[commit-bot: I haz the power, 2017-03-20 16:01:16]

Dry run: This issue passed the CQ dry run.

[dmazzoni, 2017-03-20 16:27:13]

On 2017/03/20 15:55:10, nektarios wrote:
> 1.
> +      override_device_scale_factor_for_testing_(false) {}
> I thought that override is for actions and overwrite is for values. So,
> shouldn't it be
> +      overwrite_device_scale_factor_for_testing_(false) {}
> See also other related places in the patch.

Overwrite wouldn't mean the right thing, though. I changed it to
use_custom_device_scale_factor_for_testing.

> 2. Is there an event that fires when the scale factor changes. Perhaps we can
> watch for that? Otherwise it's fine as is.

Yes, but the right one is on aura::WindowDelegate, so it'd be a lot of work
to plumb through and then it's not even clear what we'd do immediately in
response to it. I think changing the device scale factor is extremely rare
so this is a reasonable place to check it.

> 3. In BrowserAccessibilityWin I left a TODO
> // TODO(nektar): Figure out the current value of pixels per inch.
> Could you somehow deal with this TODO as part of this patch? I.e. is there a
way
> to tell the pixels per inch from the scale factor?

I think this is one of those weird areas where we're not supposed to use the
actual device pixels per inch. Basically nobody ever wrote their apps to adapt
to different device resolutions, so if you have a high-resolution monitor, the
device scale factor lets you have 2 real pixels in each dimension for every
virtual pixel. So a font size of 14px really means 28px in reality but we call
it
14px or about 12pt, regardless of the device scale factor. The short version is
it's complicated and I think the code may be correct already, but we could check
with someone else separately.

[dmazzoni, 2017-03-20 16:34:03]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2017-03-20 16:34:03]

The patchset sent to the CQ was uploaded after l-g-t-m from nektar@chromium.org,
avi@chromium.org
Link to the patchset: https://codereview.chromium.org/2759903002/#ps40001
(title: "Rename to use_custom_device_scale_factor_for_testing")

[commit-bot: I haz the power, 2017-03-20 16:34:28]

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, 2017-03-20 18:09:58]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490027643315270,
"parent_rev": "226b1ce56b1ad0e128d024b89cc5389059901bc8", "commit_rev":
"b1d3174a5e465867b3ae541ba545a47e11ace4b2"}

[commit-bot: I haz the power, 2017-03-20 18:10:45]

Description was changed from

==========
Accessible bounds should include device scale factor on Windows.

When reporting the bounding box of an object from the web
via an accessibility API on Windows, we need to take the device
scale factor into account. Other platforms either don't need this
(Mac) or already take it into account elsewhere (Android).

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

to

==========
Accessible bounds should include device scale factor on Windows.

When reporting the bounding box of an object from the web
via an accessibility API on Windows, we need to take the device
scale factor into account. Other platforms either don't need this
(Mac) or already take it into account elsewhere (Android).

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

Review-Url: https://codereview.chromium.org/2759903002
Cr-Commit-Position: refs/heads/master@{#458120}
Committed:
https://chromium.googlesource.com/chromium/src/+/b1d3174a5e465867b3ae541ba545...
==========

[commit-bot: I haz the power, 2017-03-20 18:10:46]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/b1d3174a5e465867b3ae541ba545...