Refactor BubbleDelegateView::use_focuseless()

chrome/browser/ui/views/location_bar/zoom_bubble_view.cc

Index: chrome/browser/ui/views/location_bar/zoom_bubble_view.cc
diff --git a/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc b/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc
index 3ab1d2f773bc0faf95a00ffe0d1dd5f24b472a52..426c9ebd1834ec017100351fd7fe27f78e50dcfe 100644
--- a/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc
+++ b/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc
@@ -101,10 +101,10 @@ void ZoomBubbleView::ShowBubble(content::WebContents* web_contents,
   if (is_fullscreen)
     zoom_bubble_->AdjustForFullscreen(browser_view->GetBoundsInScreen());
 
-  if (zoom_bubble_->use_focusless())
-    zoom_bubble_->GetWidget()->ShowInactive();
-  else
+  if (zoom_bubble_->can_activate())

[msw, 2014/07/23 18:57:00]

nit: I think this can just call Show and it'll respect can_activate.
(that's if can_activate is set properly, see related comments)

[Dan Beam, 2014/07/23 23:03:34]

the last time i checked, can_active() { return false; } + Show() still hijacked
focus on some platforms (or maybe it was some cases?).  i can't remember exactly
why.

[vasilii, 2014/07/24 16:32:14]

This line is my typo for sure.
Dan, I touched this problem in the description.

     zoom_bubble_->GetWidget()->Show();
+  else
+    zoom_bubble_->GetWidget()->ShowInactive();
 }
 
 // static
@@ -139,7 +139,6 @@ ZoomBubbleView::ZoomBubbleView(
       immersive_mode_controller_(immersive_mode_controller) {
   // Compensate for built-in vertical padding in the anchor view's image.
   set_anchor_view_insets(gfx::Insets(5, 0, 5, 0));
-  set_use_focusless(auto_close);

[msw, 2014/07/23 18:57:00]

You've removed all the calls to set_use_focusless / set_can_activate for this
bubble, which is likely problematic. I think the zoom bubble needs to
ShowInactive on the [Ctrl]-[+] accelerator (to avoid altering focus), but it
needs to take activation when the user clicks that timed bubble, and when the
user invokes the magnifying glass page action icon by click or keyboard.

[Dan Beam, 2014/07/23 23:03:34]

correct, ctrl+/- should not move focus away from the web contents

[vasilii, 2014/07/24 16:32:13]

ctrl+ doesn't remove focus, I checked manually.

[Dan Beam, 2014/07/24 23:24:51]

did you check on linux chromeos, linux aura, and windows?

[vasilii, 2014/07/25 15:27:43]

Yeah.

   set_notify_enter_exit_on_child(true);
 
   // Add observers to close the bubble if the fullscreen state or immersive
@@ -250,12 +249,10 @@ void ZoomBubbleView::OnExtensionIconImageChanged(
 }
 
 void ZoomBubbleView::OnMouseEntered(const ui::MouseEvent& event) {
-  set_use_focusless(false);
   StopTimer();
 }
 
 void ZoomBubbleView::OnMouseExited(const ui::MouseEvent& event) {
-  set_use_focusless(auto_close_);
   StartTimerIfNecessary();
 }