MacViews: Allows the toolkit-views Manage Passwords Dialog to be used

chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc

Index: chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc
diff --git a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc
index cb65bf63633e5c179ec583f4b7dadb493d1a41ed..1efcaf62f54d2845f0df2dd900ebf994c8b7137a 100644
--- a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc
+++ b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc
@@ -8,13 +8,14 @@
 #include "base/metrics/user_metrics.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/timer/timer.h"
+#include "chrome/browser/platform_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/exclusive_access/fullscreen_controller.h"
 #include "chrome/browser/ui/passwords/password_dialog_prompts.h"
 #include "chrome/browser/ui/passwords/passwords_model_delegate.h"
-#include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/harmony/chrome_layout_provider.h"
 #include "chrome/browser/ui/views/passwords/credentials_item_view.h"
 #include "chrome/browser/ui/views/passwords/credentials_selection_view.h"
@@ -25,6 +26,7 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/base/ui_features.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/native_theme/native_theme.h"
@@ -40,6 +42,10 @@
 #include "ui/views/layout/layout_constants.h"
 #include "ui/views/widget/widget.h"
 
+#if !defined(OS_MACOSX) || BUILDFLAG(MAC_VIEWS_BROWSER)
+#include "chrome/browser/ui/views/frame/browser_view.h"
+#endif
+
 #if defined(OS_WIN)
 #include "chrome/browser/ui/views/desktop_ios_promotion/desktop_ios_promotion_bubble_view.h"
 #endif
@@ -236,10 +242,16 @@ ManagePasswordsBubbleView::AutoSigninView::AutoSigninView(
   Browser* browser =
       chrome::FindBrowserWithWebContents(parent_->web_contents());
   DCHECK(browser);
+
+// On MacOS the BrowserView should but does not yet exist. Auto-close of the
+// auto-sign-in dialog is not implemented for a Cocoa browser when a bubble is
+// shown in a non-active browser window.

[tapted, 2017/04/19 03:31:06]

perhaps add "This matches the behaviour on Mac when using a Cocoa bubble."

(looking for words that don't suggest we may be regressing this behaviour by
switching to Views bubbles)

[varkha, 2017/04/19 08:34:17]

Done.

+// See http://crbug.com/468281.
+#if !defined(OS_MACOSX) || BUILDFLAG(MAC_VIEWS_BROWSER)
   BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
   observed_browser_.Add(browser_view->GetWidget());
-
-  if (browser_view->IsActive())
+#endif
+  if (browser->window()->IsActive())
     timer_.Start(FROM_HERE, GetTimeout(), this, &AutoSigninView::OnTimer);
 }
 
@@ -708,9 +720,10 @@ void ManagePasswordsBubbleView::ShowBubble(
   DCHECK(!manage_passwords_bubble_ ||
          !manage_passwords_bubble_->GetWidget()->IsVisible());
 
+  views::View* anchor_view = nullptr;
+#if !defined(OS_MACOSX) || BUILDFLAG(MAC_VIEWS_BROWSER)
   BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
   bool is_fullscreen = browser_view->IsFullscreen();
-  views::View* anchor_view = nullptr;
   if (!is_fullscreen) {
     if (ui::MaterialDesignController::IsSecondaryUiMaterial()) {
       anchor_view = browser_view->GetLocationBarView();
@@ -719,9 +732,13 @@ void ManagePasswordsBubbleView::ShowBubble(
           browser_view->GetLocationBarView()->manage_passwords_icon_view();
     }
   }
-  manage_passwords_bubble_ = new ManagePasswordsBubbleView(
-      web_contents, anchor_view, reason);
+#endif
 
+  new ManagePasswordsBubbleView(web_contents, anchor_view, gfx::Point(),

[tapted, 2017/04/19 03:31:06]

the gfx::Point() here doesn't look right - where does the Cocoa browser pass in
the anchor? Do we need an overload of ManagePasswordsBubbleView::ShowBubble()
that the Cocoa browser calls?  (maybe all the methods Mac needs call the
ManagePasswordsBubbleView() constructor directly, and we can NOTREACHED() here)

[varkha, 2017/04/19 08:34:17]

This code path is now only used not on Mac views and the |anchor_point| there
does not matter.
Made the test calling the bubble through
TabDialogsViewsMac::ShowManagePasswordsBubble().

+                                reason);
+  DCHECK(manage_passwords_bubble_);
+
+#if !defined(OS_MACOSX) || BUILDFLAG(MAC_VIEWS_BROWSER)
   if (is_fullscreen)
     manage_passwords_bubble_->set_parent_window(web_contents->GetNativeView());
 
@@ -737,6 +754,14 @@ void ManagePasswordsBubbleView::ShowBubble(
     manage_passwords_bubble_->AdjustForFullscreen(
         browser_view->GetBoundsInScreen());
   }
+#else
+  gfx::NativeView parent =
+      platform_util::GetViewForWindow(browser->window()->GetNativeWindow());
+  DCHECK(parent);
+  manage_passwords_bubble_->set_arrow(views::BubbleBorder::TOP_RIGHT);
+  manage_passwords_bubble_->set_parent_window(parent);
+  views::BubbleDialogDelegateView::CreateBubble(manage_passwords_bubble_);
+#endif
 
   manage_passwords_bubble_->ShowForReason(reason);
 }
@@ -761,8 +786,9 @@ content::WebContents* ManagePasswordsBubbleView::web_contents() const {
 ManagePasswordsBubbleView::ManagePasswordsBubbleView(
     content::WebContents* web_contents,
     views::View* anchor_view,
+    const gfx::Point& anchor_point,
     DisplayReason reason)
-    : LocationBarBubbleDelegateView(anchor_view, web_contents),
+    : LocationBarBubbleDelegateView(anchor_view, anchor_point, web_contents),
       model_(PasswordsModelDelegateFromWebContents(web_contents),
              reason == AUTOMATIC ? ManagePasswordsBubbleModel::AUTOMATIC
                                  : ManagePasswordsBubbleModel::USER_ACTION),
@@ -773,11 +799,12 @@ ManagePasswordsBubbleView::ManagePasswordsBubbleView(
   set_title_margins(gfx::Insets(ChromeLayoutProvider::Get()->GetDistanceMetric(
                                     DISTANCE_PANEL_CONTENT_MARGIN),
                                 side_margin, 0, side_margin));
+  manage_passwords_bubble_ = this;
 }
 
 ManagePasswordsBubbleView::~ManagePasswordsBubbleView() {
   if (manage_passwords_bubble_ == this)
-    manage_passwords_bubble_ = NULL;
+    manage_passwords_bubble_ = nullptr;
 }
 
 views::View* ManagePasswordsBubbleView::GetInitiallyFocusedView() {