Rename WindowTreeHost functions to indicate pixels/dips.

Rename WindowTreeHost functions to indicate pixels/dips.

BUG=664212
TEST=covered by tests

Committed: https://crrev.com/8077b13220db6deab5e931e9917e93c870c69a7c
Cr-Commit-Position: refs/heads/master@{#435106}

[riajiang, 2016-11-23 22:17:06]

Patchset #1 (id:1) has been deleted

[riajiang, 2016-11-23 22:18:06]

riajiang@chromium.org changed reviewers:
+ sadrul@chromium.org, sky@chromium.org

[riajiang, 2016-11-23 22:18:07]

Hi sadrul@ and sky@, PTAL. Thanks!

[sky, 2016-11-23 23:04:48]

https://codereview.chromium.org/2525113002/diff/20001/ui/aura/window_tree_host.h
File ui/aura/window_tree_host.h (right):

https://codereview.chromium.org/2525113002/diff/20001/ui/aura/window_tree_hos...
ui/aura/window_tree_host.h:130: void MoveCursorToHostLocationInPixel(
You are patch uses Pixels. Please be consistent. Personally I prefer pixels.

[riajiang, 2016-11-23 23:16:44]

https://codereview.chromium.org/2525113002/diff/20001/ui/aura/window_tree_host.h
File ui/aura/window_tree_host.h (right):

https://codereview.chromium.org/2525113002/diff/20001/ui/aura/window_tree_hos...
ui/aura/window_tree_host.h:130: void MoveCursorToHostLocationInPixel(
On 2016/11/23 23:04:48, sky wrote:
> You are patch uses Pixels. Please be consistent. Personally I prefer pixels.

I was using pixel/pixels based on if the noun is single/plural so BoundsInPixels
and LocationInPixel... hmm it is a bit weird. Do you think it should be pixels
everywhere? Also is DIP okay?

[sky, 2016-11-23 23:28:05]

pixels everywhere, and as DIP can mean 'Device Independent Pixels' no DIPs.

On Wed, Nov 23, 2016 at 3:16 PM,  <riajiang@chromium.org> wrote:
>
>
https://codereview.chromium.org/2525113002/diff/20001/ui/aura/window_tree_host.h
> File ui/aura/window_tree_host.h (right):
>
>
https://codereview.chromium.org/2525113002/diff/20001/ui/aura/window_tree_hos...
> ui/aura/window_tree_host.h:130: void MoveCursorToHostLocationInPixel(
> On 2016/11/23 23:04:48, sky wrote:
>> You are patch uses Pixels. Please be consistent. Personally I prefer
> pixels.
>
> I was using pixel/pixels based on if the noun is single/plural so
> BoundsInPixels and LocationInPixel... hmm it is a bit weird. Do you
> think it should be pixels everywhere? Also is DIP okay?
>
> https://codereview.chromium.org/2525113002/

-- 
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.

[riajiang, 2016-11-24 16:54:08]

On 2016/11/23 23:28:05, sky wrote:
> pixels everywhere, and as DIP can mean 'Device Independent Pixels' no DIPs.
> 
> On Wed, Nov 23, 2016 at 3:16 PM,  <mailto:riajiang@chromium.org> wrote:
> >
> >
>
https://codereview.chromium.org/2525113002/diff/20001/ui/aura/window_tree_host.h
> > File ui/aura/window_tree_host.h (right):
> >
> >
>
https://codereview.chromium.org/2525113002/diff/20001/ui/aura/window_tree_hos...
> > ui/aura/window_tree_host.h:130: void MoveCursorToHostLocationInPixel(
> > On 2016/11/23 23:04:48, sky wrote:
> >> You are patch uses Pixels. Please be consistent. Personally I prefer
> > pixels.
> >
> > I was using pixel/pixels based on if the noun is single/plural so
> > BoundsInPixels and LocationInPixel... hmm it is a bit weird. Do you
> > think it should be pixels everywhere? Also is DIP okay?
> >
> > https://codereview.chromium.org/2525113002/
> 
> -- 
> 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 mailto:chromium-reviews+unsubscribe@chromium.org.
> 

Changed to use pixels.

[sky, 2016-11-26 17:09:54]

Sorry, a couple more suggestions. If you agree, make sure Sadrul agrees.

https://codereview.chromium.org/2525113002/diff/60001/ui/aura/window_tree_host.h
File ui/aura/window_tree_host.h (right):

https://codereview.chromium.org/2525113002/diff/60001/ui/aura/window_tree_hos...
ui/aura/window_tree_host.h:104: void
ConvertPointFromDIPToNativeScreen(gfx::Point* point) const;
I find the use of screen and host in these function names confusing. This
function is really ConvertPointFromHostDIPToScreenPixels, but I'm not sure
'host' really adds much. WDYT of
ConvertDIPToScreenInPixels/ConvertScreenPixelsToDIP?

for the host ones ConvertDIPToPixels() and ConvertPixelsToDIP.

https://codereview.chromium.org/2525113002/diff/60001/ui/aura/window_tree_hos...
ui/aura/window_tree_host.h:131: void MoveCursorToHostLocationInPixels(
Similarly here remove host, just MoveCursorToLocationInPixels.

https://codereview.chromium.org/2525113002/diff/60001/ui/aura/window_tree_hos...
ui/aura/window_tree_host.h:213: virtual void MoveCursorToNativeInPixels(
MoveCursorToScreenLocationInPixels?

[riajiang, 2016-11-29 16:53:19]

On 2016/11/26 17:09:54, sky wrote:
> Sorry, a couple more suggestions. If you agree, make sure Sadrul agrees.
> 
>
https://codereview.chromium.org/2525113002/diff/60001/ui/aura/window_tree_host.h
> File ui/aura/window_tree_host.h (right):
> 
>
https://codereview.chromium.org/2525113002/diff/60001/ui/aura/window_tree_hos...
> ui/aura/window_tree_host.h:104: void
> ConvertPointFromDIPToNativeScreen(gfx::Point* point) const;
> I find the use of screen and host in these function names confusing. This
> function is really ConvertPointFromHostDIPToScreenPixels, but I'm not sure
> 'host' really adds much. WDYT of
> ConvertDIPToScreenInPixels/ConvertScreenPixelsToDIP?
> 
> for the host ones ConvertDIPToPixels() and ConvertPixelsToDIP.
> 
>
https://codereview.chromium.org/2525113002/diff/60001/ui/aura/window_tree_hos...
> ui/aura/window_tree_host.h:131: void MoveCursorToHostLocationInPixels(
> Similarly here remove host, just MoveCursorToLocationInPixels.
> 
>
https://codereview.chromium.org/2525113002/diff/60001/ui/aura/window_tree_hos...
> ui/aura/window_tree_host.h:213: virtual void MoveCursorToNativeInPixels(
> MoveCursorToScreenLocationInPixels?

Changed to ConvertDIPToScreenInPixels/ConvertScreenInPixelsToDIP, 
ConvertDIPToPixels/ConvertPixelsToDIP, MoveCursorToLocationInPixels,
GetLocationOnScreenInPixels
and MoveCursorToScreenLocationInPixels.

[riajiang, 2016-11-29 16:53:50]

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

[commit-bot: I haz the power, 2016-11-29 16:54:03]

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-29 17:28:12]

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

[commit-bot: I haz the power, 2016-11-29 17:28:13]

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...)

[sky, 2016-11-29 20:08:22]

LGTM - thanks for seeing this through!

[riajiang, 2016-11-29 20:17:04]

riajiang@chromium.org changed reviewers:
+ jamescook@chromium.org

[riajiang, 2016-11-29 20:17:05]

sadrul@, could you take a look at content/browser?

+jamescook@, could you take a look at extensions/shell? 

Thanks!

[sadrul, 2016-11-29 20:43:37]

lgtm

[James Cook, 2016-11-29 20:48:54]

extensions/shell lgtm

[riajiang, 2016-11-29 21:21:35]

The CQ bit was checked by riajiang@chromium.org

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

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-29 23:35:40]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1480454495223410,
"parent_rev": "138a81aaf6b569b6cc192ffa70ec7b65532cc5dc", "commit_rev":
"0a7e0acf515d6c29deec26845bbbd29ae58be5b7"}

[commit-bot: I haz the power, 2016-11-29 23:36:12]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2016-11-29 23:38:03]

Description was changed from

==========
Rename WindowTreeHost functions to indicate pixels/dips.

BUG=664212
TEST=covered by tests
==========

to

==========
Rename WindowTreeHost functions to indicate pixels/dips.

BUG=664212
TEST=covered by tests

Committed: https://crrev.com/8077b13220db6deab5e931e9917e93c870c69a7c
Cr-Commit-Position: refs/heads/master@{#435106}
==========

[commit-bot: I haz the power, 2016-11-29 23:38:04]

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