Update permission bubble anchor when omnibar is hidden

chrome/browser/ui/views/frame/browser_view.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/frame/browser_view.h"
 
 #include <algorithm>
 
 #include "base/auto_reset.h"
 #include "base/command_line.h"
@@ skipping 522 matching lines @@
 BrowserView* BrowserView::GetBrowserViewForBrowser(const Browser* browser) {
   return static_cast<BrowserView*>(browser->window());
 }
 
 void BrowserView::InitStatusBubble() {
   status_bubble_.reset(new StatusBubbleViews(contents_web_view_));
   contents_web_view_->SetStatusBubble(status_bubble_.get());
 }
 
 void BrowserView::InitPermissionBubbleView() {
-  std::string languages =
-      browser_->profile()->GetPrefs()->GetString(prefs::kAcceptLanguages);
   permission_bubble_view_.reset(new PermissionBubbleViewViews(
-      GetLocationBarView()->location_icon_view(), languages));
+      browser_->profile()->GetPrefs()->GetString(prefs::kAcceptLanguages)));
+  UpdatePermissionBubbleViewAnchor();
+}
+
+void BrowserView::UpdatePermissionBubbleViewAnchor() {

[msw, 2015/03/11 22:25:30]

Other bubbles (bookmark, global error, avatar/profile, etc.) don't have this
special behavior in fullscreen (no arrow, anchor to the fullscreen bubble or top
container). I think we ought to be consistent about the behavior of these
bubbles... please ping UX about the right behavior.

I'd generally support your solution here, but only if the same behavior were
recommended for the other bubbles (with bugs filed to follow up), and we had
some mechanism [planned] to handle overlapping bubbles (what if the permission
bubble is showing and the user hits CTRL+D for the bookmark bubble, CTRL+M for
the avatar/profile bubble, or gets a global error bubble, etc.)...

[groby-ooo-7-16, 2015/03/12 00:44:49]

Hector: The OSX version manages without modifications to the BrowserView. Can we
take the same approach on views?

If we can't, can the bubble view listen to a fullscreen change notification and
just re-query its anchor point, as opposed to hard-coding the dependency here?

[hcarmona, 2015/03/12 18:43:33]

Is there a specific person I should ping, or should I send a message to
chrome-ux@?
Unfortunately, no because in OSX when fullscreen toggles the bubbles still have
an anchor. This is necessary in order to re-anchor the permission bubbles. Most
bubbles are dismissed in the transition, but permission bubbles are special
because users should interact with them.
I was thinking that this would be a good place to send the notification because
BrowserView already knows about both the anchor point and the permission bubble.

[groby-ooo-7-16, 2015/03/12 19:27:46]

ainslie@ is looking at all things bubble. felt@ can tell you how important it is
to push this for permissions, even if we don't unify all the bubbles yet. (My
take is that it's important enough to justify a one-off)
I'll defer to msw@ on that - he's more familiar with the views side of things.

+  if (browser_->SupportsWindowFeature(Browser::FEATURE_LOCATIONBAR)) {
+    permission_bubble_view_->SetAnchor(
+        GetLocationBarView()->location_icon_view(),
+        views::BubbleBorder::TOP_LEFT);
+  } else {
+    views::View* view = NULL;

[msw, 2015/03/11 22:25:30]

nit: init |view| to |top_container_| to avoid the else below.

[hcarmona, 2015/03/12 18:43:33]

Done.

+
+    if (IsFullscreenBubbleVisible())
+      view = exclusive_access_bubble_->GetView();
+    else
+      view = top_container_;
+
+    permission_bubble_view_->SetAnchor(view, views::BubbleBorder::NONE);
+  }
 }
 
 gfx::Rect BrowserView::GetToolbarBounds() const {
   gfx::Rect toolbar_bounds(toolbar_->bounds());
   if (toolbar_bounds.IsEmpty())
     return toolbar_bounds;
   // The apparent toolbar edges are outside the "real" toolbar edges.
   toolbar_bounds.Inset(-views::NonClientFrameView::kClientEdgeThickness, 0);
   return toolbar_bounds;
 }
@@ skipping 1727 matching lines @@
   }
 
   // Undo our anti-jankiness hacks and force a re-layout. We also need to
   // recompute the height of the infobar top arrow because toggling in and out
   // of fullscreen changes it. Calling ToolbarSizeChanged() will do both these
   // things since it computes the arrow height directly and forces a layout
   // indirectly via UpdateUIForContents(). Reset |in_process_fullscreen_| in
   // order to let the layout occur.
   in_process_fullscreen_ = false;
   ToolbarSizeChanged(false);
+
+  UpdatePermissionBubbleViewAnchor();
 }
 
 bool BrowserView::ShouldUseImmersiveFullscreenForUrl(const GURL& url) const {
   // Kiosk mode needs the whole screen, and if we're not in an Ash desktop
   // immersive fullscreen doesn't exist.
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kKioskMode) ||
       browser()->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH) {
     return false;
   }
 
@@ skipping 300 matching lines @@
   return immersive_mode_controller()->IsEnabled();
 }
 
 views::Widget* BrowserView::GetBubbleAssociatedWidget() {
   return GetWidget();
 }
 
 gfx::Rect BrowserView::GetTopContainerBoundsInScreen() {
   return top_container_->GetBoundsInScreen();
 }