MacViews: Allows the toolkit-views Device Chooser bubble to be used

chrome/browser/ui/cocoa/permission_bubble/chooser_bubble_ui_views_mac.mm

+// Copyright 2017 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 "base/memory/ptr_util.h"
+#include "chrome/browser/chooser_controller/chooser_controller.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/cocoa/browser_window_controller.h"
+#import "chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h"
+#import "chrome/browser/ui/cocoa/permission_bubble/chooser_bubble_ui_cocoa.h"
+#include "chrome/browser/ui/permission_bubble/chooser_bubble_delegate.h"
+#include "chrome/browser/ui/views/permission_bubble/chooser_bubble_ui_view.h"
+#import "ui/base/cocoa/cocoa_base_utils.h"
+#include "ui/base/material_design/material_design_controller.h"
+#include "ui/base/ui_features.h"
+#import "ui/gfx/mac/coordinate_conversion.h"
+
+namespace {
+
+// Takes in the parent window, which should be a BrowserWindow, and gets the
+// proper anchor point for the bubble. The returned point is in screen
+// coordinates.
+NSPoint AnchorPointForWindow(NSWindow* parent) {

[varkha, 2017/05/02 12:25:48]

Self review: simplify with inlines.

[varkha, 2017/05/03 05:01:20]

Acknowledged.

+  BrowserWindowController* controller = [parent windowController];
+  NSPoint origin = NSZeroPoint;
+  if ([controller isKindOfClass:[BrowserWindowController class]]) {
+    LocationBarViewMac* location_bar = [controller locationBarBridge];
+    if (location_bar) {
+      NSPoint bubble_point = location_bar->GetPageInfoBubblePoint();
+      origin = ui::ConvertPointFromWindowToScreen(parent, bubble_point);
+    }

[tapted, 2017/05/03 01:10:53]

Do we need an else?

Maybe we can move this method to its own header (bubble_anchor_helper_views.h
perhaps)

There's logic in chooser_bubble_ui_cocoa.mm that falls back to centering when
there is no location bar - it should be able to call the helper too.

(There's logic in permission_bubble_controller.mm too... (except it falls back
to "left of screen"))

We can probably do a similar thing for things anchored on the right of the
location bar, but we can worry about that later.

[varkha, 2017/05/03 05:01:20]

Acknowledged. I'll look into this.

+  }
+  return origin;
+}
+
+}  // namespace
+
+#if !BUILDFLAG(MAC_VIEWS_BROWSER)

[tapted, 2017/05/03 01:10:53]

Can we exclude the entire file under the mac_views_browser build in .gn?

[varkha, 2017/05/03 05:01:20]

Please see if the refactoring I've done makes sense. It's similar to the CL
you've linked and avoids #ifdefs.

+std::unique_ptr<BubbleUi> ChooserBubbleDelegate::BuildBubbleUi() {
+  if (ui::MaterialDesignController::IsSecondaryUiMaterial()) {

[varkha, 2017/05/02 12:25:48]

Self review: simplify by reversing the condition and inlining const temp vars.

[varkha, 2017/05/03 05:01:20]

Done.

+    NSWindow* parent = browser_->window()->GetNativeWindow();
+    gfx::Point anchor_point =
+        gfx::ScreenPointFromNSPoint(AnchorPointForWindow(parent));
+    return base::MakeUnique<ChooserBubbleUiView>(
+        browser_, std::move(chooser_controller_), anchor_point);
+  }
+  return base::MakeUnique<ChooserBubbleUiCocoa>(browser_,
+                                                std::move(chooser_controller_));
+}
+#endif