Correctly move permission bubbles after entering / exiting fullscreen.

Correctly move permission bubbles after entering / exiting fullscreen.

When full screen is entered or exited the position of the bubble arrow
may need to be updated. This change updates the location, and ensures
the bubble is repainted when the arrow location is updated.

BUG=614928
Committed: https://crrev.com/2c657c1722cfc92eb307341859a294ff64d6d40a
Cr-Commit-Position: refs/heads/master@{#397314}

[benwells, 2016-06-02 01:09:05]

benwells@chromium.org changed reviewers:
+ tapted@chromium.org

[benwells, 2016-06-02 01:09:06]

fyi this would be to merge into 52 ... I could probably do it in a safer way
(i.e. not way down in the bubble view), let me know if you think i should.

[tapted, 2016-06-02 01:23:07]

https://codereview.chromium.org/2036503002/diff/1/chrome/browser/ui/cocoa/web...
File chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
(right):

https://codereview.chromium.org/2036503002/diff/1/chrome/browser/ui/cocoa/web...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:305:
[[self bubble] setArrowLocation:[self getExpectedArrowLocation]];
I think this should be in - (void)updateAnchorPosition

and... parentWindowDidResize: should just call updateAnchorPosition?


(Also browser_window_controller_private.mm should _already_ be invoking
updateAnchorPosition on fullscreen transitions since it does


  // Will update the location of the permission bubble when showing/hiding the
  // top level toolbar in fullscreen.
  PermissionBubbleManager* manager = [self permissionBubbleManager];
  if (manager)
    manager->UpdateAnchorPosition();

That is, parentWindowDidResize: is probably completely unneeded, but there might
be something obscure, so I think it's fine to keep it for now, but just have it
call updateAnchorPosition)

[benwells, 2016-06-02 01:55:51]

https://codereview.chromium.org/2036503002/diff/1/chrome/browser/ui/cocoa/web...
File chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
(right):

https://codereview.chromium.org/2036503002/diff/1/chrome/browser/ui/cocoa/web...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:305:
[[self bubble] setArrowLocation:[self getExpectedArrowLocation]];
On 2016/06/02 01:23:07, tapted wrote:
> I think this should be in - (void)updateAnchorPosition
> 
> and... parentWindowDidResize: should just call updateAnchorPosition?
> 
> 
> (Also browser_window_controller_private.mm should _already_ be invoking
> updateAnchorPosition on fullscreen transitions since it does
> 
> 
>   // Will update the location of the permission bubble when showing/hiding the
>   // top level toolbar in fullscreen.
>   PermissionBubbleManager* manager = [self permissionBubbleManager];
>   if (manager)
>     manager->UpdateAnchorPosition();
> 
> That is, parentWindowDidResize: is probably completely unneeded, but there
might
> be something obscure, so I think it's fine to keep it for now, but just have
it
> call updateAnchorPosition)

Yeap ... I thought about all of that but wanted to keep the patch as minimal as
possible, and was worried about breaking things. I'll play with moving it around
and see what the impact is like.

[benwells, 2016-06-02 05:26:03]

https://codereview.chromium.org/2036503002/diff/1/chrome/browser/ui/cocoa/web...
File chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
(right):

https://codereview.chromium.org/2036503002/diff/1/chrome/browser/ui/cocoa/web...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:305:
[[self bubble] setArrowLocation:[self getExpectedArrowLocation]];
On 2016/06/02 01:55:51, benwells wrote:
> On 2016/06/02 01:23:07, tapted wrote:
> > I think this should be in - (void)updateAnchorPosition
> > 
> > and... parentWindowDidResize: should just call updateAnchorPosition?
> > 
> > 
> > (Also browser_window_controller_private.mm should _already_ be invoking
> > updateAnchorPosition on fullscreen transitions since it does
> > 
> > 
> >   // Will update the location of the permission bubble when showing/hiding
the
> >   // top level toolbar in fullscreen.
> >   PermissionBubbleManager* manager = [self permissionBubbleManager];
> >   if (manager)
> >     manager->UpdateAnchorPosition();
> > 
> > That is, parentWindowDidResize: is probably completely unneeded, but there
> might
> > be something obscure, so I think it's fine to keep it for now, but just have
> it
> > call updateAnchorPosition)
> 
> Yeap ... I thought about all of that but wanted to keep the patch as minimal
as
> possible, and was worried about breaking things. I'll play with moving it
around
> and see what the impact is like.

Done.

[tapted, 2016-06-02 05:39:28]

lgtm

[benwells, 2016-06-02 05:53:29]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2016-06-02 05:54:02]

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

[commit-bot: I haz the power, 2016-06-02 06:33:15]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-02 06:34:26]

Description was changed from

==========
Correctly move permission bubbles after entering / exiting fullscreen.

When full screen is entered or exited the position of the bubble arrow
may need to be updated. This change updates the location, and ensures
the bubble is repainted when the arrow location is updated.

BUG=614928
==========

to

==========
Correctly move permission bubbles after entering / exiting fullscreen.

When full screen is entered or exited the position of the bubble arrow
may need to be updated. This change updates the location, and ensures
the bubble is repainted when the arrow location is updated.

BUG=614928

Committed: https://crrev.com/2c657c1722cfc92eb307341859a294ff64d6d40a
Cr-Commit-Position: refs/heads/master@{#397314}
==========

[commit-bot: I haz the power, 2016-06-02 06:34:27]

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