HiDPI: Fix extension bubble size

HiDPI: Fix extension bubble size

The size of extension bubbles was wrong in HiDPI mode.

The problem was that we were using convertRectToBase. As per Avi's PSA, we should be using convertRect:ToView:. See:
https://groups.google.com/a/chromium.org/group/chromium-dev/browse_thread/thread/6b989dcbb79e9ba/029eeb633c5287cc

I did a search and it looks like we made the same mistake in the tab dragging code. Before my fix it was impossible to drag a tab from one window to the right of a tab in another window. This fixes that as well.

BUG=132762
TEST=Ran in HiDPI mode on 10.7. Without my fix the extension bubble was 4x the normal size. With my fix the extension bubble was correctly 2x the normal size.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=142347

[Nico, 2012-06-15 02:21:48]

Lgtm

Thanks!

(Do we call convertrecttobase anywhere else?)
On Jun 14, 2012 6:52 PM, <sail@chromium.org> wrote:

> Reviewers: Nico, rsesek, Avi,
>
> Description:
> HiDPI: Fix extension bubble size
>
> The size of extension bubbles was wrong in HiDPI mode.
>
> The problem was that we were using convertRectToBase. As per Avi's PSA, we
> should be using convertRect:ToView:. See:
> https://groups.google.com/a/**chromium.org/group/chromium-**
>
dev/browse_thread/thread/**6b989dcbb79e9ba/**029eeb633c5287cc<https://groups.google.com/a/chromium.org/group/chromium-dev/browse_thread/thread/6b989dcbb79e9ba/029eeb633c5287cc>
>
> I did a search and it looks like we made the same mistake in the tab
> dragging
> code. Before my fix it was impossible to drag a tab from one window to the
> right
> of a tab in another window. This fixes that as well.
>
> BUG=132762
> TEST=Ran in HiDPI mode on 10.7. Without my fix the extension bubble was 4x
> the
> normal size. With my fix the extension bubble was correctly 2x the correct
> size.
>
>
> Please review this at
https://chromiumcodereview.**appspot.com/10540173/<https://chromiumcodereview...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>  M
chrome/browser/ui/cocoa/browse**r_window_controller.mm<http://browser_window_controller.mm>
>  M
chrome/browser/ui/cocoa/**extensions/extension_popup_**controller.mm<http://extension_popup_controller.mm>
>
>
> Index:
chrome/browser/ui/cocoa/browse**r_window_controller.mm<http://browser_window_controller.mm>
> diff --git
a/chrome/browser/ui/cocoa/brow**ser_window_controller.mm<http://browser_window_controller.mm>b/chrome/browser/ui/cocoa/
> brow**ser_window_controller.mm <http://browser_window_controller.mm>
> index 4ed02fd79e5cf17e3a3b126bd8277f**ebc5228e1f..**
> b1fb309e22d86cf58fdf6ceec72405**da17bdc286 100644
> ---
a/chrome/browser/ui/cocoa/brow**ser_window_controller.mm<http://browser_window_controller.mm>
> +++
b/chrome/browser/ui/cocoa/brow**ser_window_controller.mm<http://browser_window_controller.mm>
> @@ -1323,7 +1323,7 @@ enum {
>
>   // Pushes tabView's frame back inside the tabstrip.
>   NSSize tabOverflow =
> -      [self overflowFrom:[tabStrip convertRectToBase:**sourceTabRect]
> +      [self overflowFrom:[tabStrip convertRect:sourceTabRect toView:nil]
>                       to:[tabStrip frame]];
>   NSRect tabRect = NSOffsetRect(sourceTabRect,
>                                 -tabOverflow.width, -tabOverflow.height);
> Index: chrome/browser/ui/cocoa/**extensions/extension_popup_**
> controller.mm <http://extension_popup_controller.mm>
> diff --git a/chrome/browser/ui/cocoa/**extensions/extension_popup_**
> controller.mm
<http://extension_popup_controller.mm>b/chrome/browser/ui/cocoa/
>
**extensions/extension_popup_**controller.mm<http://extension_popup_controller.mm>
> index 2cd3ffe360a9ad5527005498b0f7e2**a48b8270b5..**
> 3e724c0048666ed435af7518174d70**0008c5e02e 100644
> ---
a/chrome/browser/ui/cocoa/**extensions/extension_popup_**controller.mm<http://extension_popup_controller.mm>
> +++
b/chrome/browser/ui/cocoa/**extensions/extension_popup_**controller.mm<http://extension_popup_controller.mm>
> @@ -288,7 +288,7 @@ class DevtoolsNotificationBridge : public
> content::NotificationObserver {
>   frame.size.height += info_bubble::**kBubbleArrowHeight +
>                        info_bubble::**kBubbleCornerRadius;
>   frame.size.width += info_bubble::**kBubbleCornerRadius;
> -  frame = [extensionView_ convertRectToBase:frame];
> +  frame = [extensionView_ convertRect:frame toView:nil];
>   // Adjust the origin according to the height and width so that the arrow
> is
>   // positioned correctly at the middle and slightly down from the button.
>   NSPoint windowOrigin = self.anchorPoint;
>
>
>

[Avi (use Gerrit), 2012-06-15 02:45:22]

LGTM

When I wrote that PSA I went through the codebase and fixed them all. I then for
a year reviewed just about every code with coordinate conversion and fixed them
because no one ever got it right.

Looks like people don't learn. :(

[sail, 2012-06-15 05:11:05]

On 2012/06/15 02:21:48, Nico wrote:
> Lgtm
> 
> Thanks!
> 
> (Do we call convertrecttobase anywhere else?)

The only other reference is in render_widget_host_view_mac.mm  but I can't tell
if it's incorrect.

[commit-bot: I haz the power, 2012-06-15 05:11:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/10540173/1

[Avi (use Gerrit), 2012-06-15 06:18:28]

http://code.google.com/codesearch#search/&exact_package=chromium&q=convertrec...

Every. Single. One. Of. These. Is. Wrong.

[Avi (use Gerrit), 2012-06-15 06:23:15]

On 2012/06/15 06:18:28, Avi wrote:
>
http://code.google.com/codesearch#search/&exact_package=chromium&q=convertrec...
> 
> Every. Single. One. Of. These. Is. Wrong.

So are all of these:
http://code.google.com/codesearch#search/&exact_package=chromium&q=convertPoi...

This is legit; I wrote it:
http://code.google.com/codesearch#search/&exact_package=chromium&q=convertPoi...

This is silly:
http://src.chromium.org/svn/trunk/src/chrome/browser/ui/cocoa/nsview_addition...
Why take the reciprocal of convertSizeToBase when you can use
convertSizeFromBase?

[commit-bot: I haz the power, 2012-06-15 07:36:17]

Change committed as 142347