[Password Manager] Close the bubble when fullscreen state gets change.

chrome/browser/ui/views/managed_full_screen_bubble_delegate_view.cc

Index: chrome/browser/ui/views/managed_full_screen_bubble_delegate_view.cc
diff --git a/chrome/browser/ui/views/managed_full_screen_bubble_delegate_view.cc b/chrome/browser/ui/views/managed_full_screen_bubble_delegate_view.cc
new file mode 100644
index 0000000000000000000000000000000000000000..312679e12b8f8cf9c1c7dd9e38cf95b00468dfa6
--- /dev/null
+++ b/chrome/browser/ui/views/managed_full_screen_bubble_delegate_view.cc
@@ -0,0 +1,70 @@
+// Copyright 2014 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/managed_full_screen_bubble_delegate_view.h"
+
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/fullscreen/fullscreen_controller.h"
+#include "content/public/browser/notification_source.h"
+#include "content/public/browser/web_contents.h"

[vasilii, 2014/12/17 13:40:12]

Is it needed?

[Pritam Nikam, 2014/12/17 16:06:26]

Done.

+#include "ui/gfx/geometry/rect.h"
+
+ManagedFullScreenBubbleDelegateView::ManagedFullScreenBubbleDelegateView(
+    views::View* anchor_view,
+    content::WebContents* web_contents)
+    : BubbleDelegateView(anchor_view,
+                         anchor_view ? views::BubbleBorder::TOP_RIGHT
+                                     : views::BubbleBorder::NONE) {
+  // Register accelerator key F11.
+  AddAccelerator(ui::Accelerator(ui::VKEY_F11, ui::EF_NONE));

[vasilii, 2014/12/17 13:40:11]

Why do we need it?

[Pritam Nikam, 2014/12/17 16:06:26]

Cases where bubble view is having focus, F11 wont do anything, i.e. does not
toggle the browser full screen state.
I've registered F11 to capture full screen state change event, however  I'm not
sure whether the counter action i.e. closing a bubble is right or wrong here.

[vasilii, 2014/12/18 16:16:22]

Indeed, but closing the bubble on F11 isn't what users expect. In this patch I'd
leave the problem as is. The bubble with focus will swallow all the accelerators
not only F11. If we decide to solve it we should fix it in another way.

+
+  // Add observers to close the bubble if the fullscreen state changes.
+  Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
+  registrar_.Add(
+      this, chrome::NOTIFICATION_FULLSCREEN_CHANGED,
+      content::Source<FullscreenController>(browser->fullscreen_controller()));
+}
+
+ManagedFullScreenBubbleDelegateView::~ManagedFullScreenBubbleDelegateView() {
+}
+
+void ManagedFullScreenBubbleDelegateView::AdjustForFullscreen(

[vasilii, 2014/12/17 13:40:11]

Order of definitions should match the order of declarations in the header.

[Pritam Nikam, 2014/12/17 16:06:26]

Done.

+    const gfx::Rect& screen_bounds) {
+  if (GetAnchorView())
+    return;
+
+  // The bubble's padding from the screen edge, used in fullscreen.
+  const int kFullscreenPaddingEnd = 20;

[vasilii, 2014/12/17 13:40:12]

the constant should go to the beginning of the file.

[Pritam Nikam, 2014/12/17 16:06:26]

Done.

+  const size_t bubble_half_width = width() / 2;

[vasilii, 2014/12/17 13:40:11]

should be int.

[Pritam Nikam, 2014/12/17 16:06:26]

Done.

+  const int x_pos =
+      base::i18n::IsRTL()
+          ? screen_bounds.x() + bubble_half_width + kFullscreenPaddingEnd
+          : screen_bounds.right() - bubble_half_width - kFullscreenPaddingEnd;
+  SetAnchorRect(gfx::Rect(x_pos, screen_bounds.y(), 0, 0));
+}
+
+void ManagedFullScreenBubbleDelegateView::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  DCHECK_EQ(type, chrome::NOTIFICATION_FULLSCREEN_CHANGED);
+  GetWidget()->SetVisibilityAnimationTransition(views::Widget::ANIMATE_NONE);
+  Close();
+}
+
+void ManagedFullScreenBubbleDelegateView::Close() {
+  GetWidget()->Close();
+}
+
+bool ManagedFullScreenBubbleDelegateView::AcceleratorPressed(
+    const ui::Accelerator& accelerator) {
+  if (accelerator.key_code() == ui::VKEY_F11) {
+    Close();

[vasilii, 2014/12/17 13:40:11]

Why would F11 close the bubble?

[Pritam Nikam, 2014/12/17 16:06:26]

I'm not sure whether we should close the bubble or not.

But to keep consistent behavior of closing the bubble on full-screen toggling I
have added the Close() call here.
May be appropriate behavior would be pass F11 to root view, & through
notification observer Close() this bubble.

+    return true;
+  }
+
+  return BubbleDelegateView::AcceleratorPressed(accelerator);
+}