Prepare chrome/ for compilation with OS X 10.7 deployment target.

Prepare chrome/ for compilation with OS X 10.7 deployment target.

Replaced deprecated methods convertBaseToScreen: and convertBaseFromScreen: with
ConvertPointFromWindowToScreen and ConvertPointFromScreenToWindow.

BUG=592663
Committed: https://crrev.com/b39bbac875e4ee4f041058b95b03d4c2cc95c501
Cr-Commit-Position: refs/heads/master@{#380540}

[erikchen, 2016-03-09 00:38:25]

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

[erikchen, 2016-03-09 00:38:38]

erikchen@chromium.org changed reviewers:
+ avi@chromium.org

[erikchen, 2016-03-09 00:38:39]

avi: Please review.

[commit-bot: I haz the power, 2016-03-09 00:38:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1775223002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1775223002/40001

[erikchen, 2016-03-09 00:39:17]

erikchen@chromium.org changed reviewers:
+ thakis@chromium.org

[erikchen, 2016-03-09 00:39:18]

thakis: Please review chrome/test/base/interactive_test_utils_cocoa.mm

[commit-bot: I haz the power, 2016-03-09 00:42:25]

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

[commit-bot: I haz the power, 2016-03-09 00:42:26]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[erikchen, 2016-03-09 01:03:53]

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

[commit-bot: I haz the power, 2016-03-09 01:04:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1775223002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1775223002/60001

[commit-bot: I haz the power, 2016-03-09 02:14:25]

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

[commit-bot: I haz the power, 2016-03-09 02:14:26]

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

[Nico, 2016-03-09 02:37:05]

cs.chromium.org doesn't know about ConvertPointFromScreenToWindow yet – is that
being added elsewhere?

why not call the recommended replacement directly?

[Avi (use Gerrit), 2016-03-09 17:30:07]

On 2016/03/09 02:37:05, Nico wrote:
> http://cs.chromium.org doesn't know about ConvertPointFromScreenToWindow yet
– is that
> being added elsewhere?
> 
> why not call the recommended replacement directly?

Because there is no -[NSWindow convertPointFromScreen:] method, so Erik built
one using the -[NSWindow convertRectFromScreen:] method and named it
ConvertPointFromScreenToWindow. (And the same vice-versa.)

[Avi (use Gerrit), 2016-03-09 17:30:20]

https://codereview.chromium.org/1775223002/diff/60001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
(right):

https://codereview.chromium.org/1775223002/diff/60001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm:1043:
[button window], parentButtonGlobalFrame.origin);
parentButtonGlobalFrame = [[button window]
convertRectToScreen:parentButtonGlobalFrame];

[Nico, 2016-03-09 18:19:28]

On 2016/03/09 17:30:07, Avi wrote:
> On 2016/03/09 02:37:05, Nico wrote:
> > http://cs.chromium.org doesn't know about ConvertPointFromScreenToWindow yet
> – is that
> > being added elsewhere?
> > 
> > why not call the recommended replacement directly?
> 
> Because there is no -[NSWindow convertPointFromScreen:] method, so Erik built
> one using the -[NSWindow convertRectFromScreen:] method and named it
> ConvertPointFromScreenToWindow. (And the same vice-versa.)

Well, but the reason for that is because converting points to points usually
isn't what you want since a 1x1 point in lodpi becomes a 2x2 rect in retina
(which is why the old function was deprecated in the first place), right?

[Avi (use Gerrit), 2016-03-09 19:12:29]

On 2016/03/09 18:19:28, Nico wrote:
> On 2016/03/09 17:30:07, Avi wrote:
> > On 2016/03/09 02:37:05, Nico wrote:
> > > http://cs.chromium.org doesn't know about ConvertPointFromScreenToWindow
yet
> > – is that
> > > being added elsewhere?
> > > 
> > > why not call the recommended replacement directly?
> > 
> > Because there is no -[NSWindow convertPointFromScreen:] method, so Erik
built
> > one using the -[NSWindow convertRectFromScreen:] method and named it
> > ConvertPointFromScreenToWindow. (And the same vice-versa.)
> 
> Well, but the reason for that is because converting points to points usually
> isn't what you want since a 1x1 point in lodpi becomes a 2x2 rect in retina
> (which is why the old function was deprecated in the first place), right?

No. A point is a point, a location. A rect is a rect.

If we have "location of popup", that's a point, lo- or hi-dpi. It doesn't become
a rect just because we move to hi-dpi.

[Nico, 2016-03-09 19:16:07]

Well, it depends, right? True for popup locations (assuming you want the popup
to be in the lower-left pixel of the 4 pixels covered by your window lcoation),
less true for other things, see e.g.:

https://codereview.chromium.org/1775223002/diff/60001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
(right):

https://codereview.chromium.org/1775223002/diff/60001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm:542:
[[parentButton_ superview] convertPoint:topOfWindow toView:nil]);
Here this does (maybe) the wrong thing since it drops the height of the rect,
for example.

[Avi (use Gerrit), 2016-03-09 19:19:31]

On 2016/03/09 19:16:07, Nico wrote:
> Well, it depends, right? True for popup locations (assuming you want the popup
> to be in the lower-left pixel of the 4 pixels covered by your window
lcoation),
> less true for other things, see e.g.:
> 
>
https://codereview.chromium.org/1775223002/diff/60001/chrome/browser/ui/cocoa...
> File chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
> (right):
> 
>
https://codereview.chromium.org/1775223002/diff/60001/chrome/browser/ui/cocoa...
> chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm:542:
> [[parentButton_ superview] convertPoint:topOfWindow toView:nil]);
> Here this does (maybe) the wrong thing since it drops the height of the rect,
> for example.

These changes should be neutral to the existing functionality, from how I
understand it.

[erikchen, 2016-03-10 00:25:31]

On 2016/03/09 19:19:31, Avi wrote:
> On 2016/03/09 19:16:07, Nico wrote:
> > Well, it depends, right? True for popup locations (assuming you want the
popup
> > to be in the lower-left pixel of the 4 pixels covered by your window
> lcoation),
> > less true for other things, see e.g.:
> > 
> >
>
https://codereview.chromium.org/1775223002/diff/60001/chrome/browser/ui/cocoa...
> > File chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
> > (right):
> > 
> >
>
https://codereview.chromium.org/1775223002/diff/60001/chrome/browser/ui/cocoa...
> > chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm:542:
> > [[parentButton_ superview] convertPoint:topOfWindow toView:nil]);
> > Here this does (maybe) the wrong thing since it drops the height of the
rect,
> > for example.
> 
> These changes should be neutral to the existing functionality, from how I
> understand it.

thakis: All coordinates in view, window, and screen space are in "User space",
which is independent of DPI:
https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/CocoaD...

It is possible for an NSScreen and NSWindow to have different
backingScaleFactors, but those are not relevant to "User space" coordinate
transforms.

I don't understand your comments about conversions between a point and a rect.
At the end of the day, Cocoa needs a mechanism to convert a coordinate in window
space to a coordinate in screen space. There is no scaling, since points have no
width. The new function I added (ConvertPointFromScreenToWindow) has the same
behavior as the deprecated function convertBaseToScreen:.

I looked at the disassembly for convertBaseToScreen: and convertRectToScreen:.
Both of them just do shift the point/rect relative to the origin of the window.

[Nico, 2016-03-10 18:32:43]

After rereading
https://developer.apple.com/library/mac/releasenotes/AppKit/RN-AppKitOlderNot...
I agree with y'all, thanks for your patience.

lgtm, but the test failure on the trybot looks like it's probably actually
caused by this patch.

https://codereview.chromium.org/1775223002/diff/60001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/tabs/tab_strip_drag_controller.mm (right):

https://codereview.chromium.org/1775223002/diff/60001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/tabs/tab_strip_drag_controller.mm:391:
convertRect:tabFrame fromView:nil];
this variable sure is getting transformed a lot

[erikchen, 2016-03-10 18:35:47]

On 2016/03/10 18:32:43, Nico wrote:
> After rereading
>
https://developer.apple.com/library/mac/releasenotes/AppKit/RN-AppKitOlderNot...
> I agree with y'all, thanks for your patience.
> 
> lgtm, but the test failure on the trybot looks like it's probably actually
> caused by this patch.
> 
>
https://codereview.chromium.org/1775223002/diff/60001/chrome/browser/ui/cocoa...
> File chrome/browser/ui/cocoa/tabs/tab_strip_drag_controller.mm (right):
> 
>
https://codereview.chromium.org/1775223002/diff/60001/chrome/browser/ui/cocoa...
> chrome/browser/ui/cocoa/tabs/tab_strip_drag_controller.mm:391:
> convertRect:tabFrame fromView:nil];
> this variable sure is getting transformed a lot

Will investigate trybot errors. They are likely real bugs exposed by this
refactor. I caught 2 mistakes in my refactor to content/:
https://codereview.chromium.org/1777483005/

[erikchen, 2016-03-10 21:39:50]

The CQ bit was checked by erikchen@chromium.org

[erikchen, 2016-03-10 21:39:51]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/1775223002/#ps120001
(title: "Compile errors. Fix null-window error.")

[commit-bot: I haz the power, 2016-03-10 21:40:25]

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

[commit-bot: I haz the power, 2016-03-11 00:56:37]

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

[commit-bot: I haz the power, 2016-03-11 00:56:38]

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

[erikchen, 2016-03-11 01:30:54]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2016-03-11 01:31:53]

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

[commit-bot: I haz the power, 2016-03-11 05:39:15]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-03-11 05:41:18]

Description was changed from

==========
Prepare chrome/ for compilation with OS X 10.7 deployment target.

Replaced deprecated methods convertBaseToScreen: and convertBaseFromScreen: with
ConvertPointFromWindowToScreen and ConvertPointFromScreenToWindow.

BUG=592663
==========

to

==========
Prepare chrome/ for compilation with OS X 10.7 deployment target.

Replaced deprecated methods convertBaseToScreen: and convertBaseFromScreen: with
ConvertPointFromWindowToScreen and ConvertPointFromScreenToWindow.

BUG=592663

Committed: https://crrev.com/b39bbac875e4ee4f041058b95b03d4c2cc95c501
Cr-Commit-Position: refs/heads/master@{#380540}
==========

[commit-bot: I haz the power, 2016-03-11 05:41:20]

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