[Mac] Hover/Active Omnibox Icon States for Secondary UI MD

chrome/browser/ui/cocoa/bubble_anchor_helper_views.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 "chrome/browser/ui/cocoa/bubble_anchor_helper_views.h"
 
 #import <Cocoa/Cocoa.h>
 
 #import "base/mac/scoped_nsobject.h"
+#import "chrome/browser/ui/cocoa/browser_window_controller.h"
+#import "chrome/browser/ui/cocoa/location_bar/location_bar_decoration.h"
+#import "chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h"
+#import "chrome/browser/ui/cocoa/location_bar/manage_passwords_decoration.h"
+#import "chrome/browser/ui/cocoa/location_bar/star_decoration.h"
 #include "ui/views/bubble/bubble_dialog_delegate.h"
 #include "ui/views/widget/widget_observer.h"
 
 namespace {
 
 // Self-deleting object that hosts Objective-C observers watching for parent
 // window resizes to reposition a bubble Widget. Deletes itself when the bubble
 // Widget closes.
 class BubbleAnchorHelper : public views::WidgetObserver {
  public:
-  explicit BubbleAnchorHelper(views::BubbleDialogDelegateView* bubble);
+  explicit BubbleAnchorHelper(views::BubbleDialogDelegateView* bubble,

[tapted, 2017/05/16 07:39:37]

nit: explicit not required

[spqchan, 2017/05/17 01:39:39]

Done.

+                              LocationBarDecoration* decoration);
 
  private:
   // Observe |name| on the bubble parent window with a block to call ReAnchor().
   void Observe(NSString* name);
 
   // Whether offset from the left of the parent window is fixed.
   bool IsMinXFixed() const {
     return views::BubbleBorder::is_arrow_on_left(bubble_->arrow());
   }
 
   // Re-positions |bubble_| so that the offset to the parent window at
   // construction time is preserved.
   void ReAnchor();
 
   // WidgetObserver:
   void OnWidgetDestroying(views::Widget* widget) override;
 
   base::scoped_nsobject<NSMutableArray> observer_tokens_;
   views::BubbleDialogDelegateView* bubble_;
   CGFloat horizontal_offset_;  // Offset from the left or right.
   CGFloat vertical_offset_;    // Offset from the top.
 
+  // The omnibox decoration that |bubble_| is anchored to.
+  LocationBarDecoration* decoration_;
+
   DISALLOW_COPY_AND_ASSIGN(BubbleAnchorHelper);
 };
 
 }  // namespace
 
-void KeepBubbleAnchored(views::BubbleDialogDelegateView* bubble) {
-  new BubbleAnchorHelper(bubble);
+LocationBarDecoration* GetManagePasswordDecoration(
+    views::BubbleDialogDelegateView* bubble) {
+  LocationBarViewMac* locationBarBridge =

[tapted, 2017/05/16 07:39:37]

nit: locationBarBridge -> location_bar  (camel case only for new declarations
between @foo/@end)

[spqchan, 2017/05/17 01:39:39]

Done.

+      [[[bubble->parent_window() window] windowController] locationBarBridge];

[tapted, 2017/05/16 07:39:37]

I don't think this will compile in the 10.11 or 10.12 SDK.

[NSWindow windowController] used to be declared to return `id`, but now it's
NSWindowController*, which won't have locationBarBridge declared on it.

I think we need a helper method like

LocationBarForBubbleParent(..) which uses

+[BrowserWindowController browserWindowControllerForWindow]

(and also checks if that returned nil)

[spqchan, 2017/05/17 01:39:39]

Done.

+  return locationBarBridge ? locationBarBridge->manage_passwords_decoration()
+                           : nullptr;
 }
 
-BubbleAnchorHelper::BubbleAnchorHelper(views::BubbleDialogDelegateView* bubble)
-    : observer_tokens_([[NSMutableArray alloc] init]), bubble_(bubble) {
+LocationBarDecoration* GetStarDecoration(
+    views::BubbleDialogDelegateView* bubble) {
+  LocationBarViewMac* locationBarBridge =
+      [[[bubble->parent_window() window] windowController] locationBarBridge];
+  return locationBarBridge ? locationBarBridge->star_decoration() : nullptr;
+}
+
+LocationBarDecoration* GetPageInfoDecoration(
+    views::BubbleDialogDelegateView* bubble) {
+  LocationBarViewMac* locationBarBridge =
+      [[[bubble->parent_window() window] windowController] locationBarBridge];
+  return locationBarBridge ? locationBarBridge->GetPageInfoDecoration()
+                           : nullptr;
+}
+
+void KeepBubbleAnchored(views::BubbleDialogDelegateView* bubble,
+                        LocationBarDecoration* decoration) {
+  new BubbleAnchorHelper(bubble, decoration);
+}
+
+BubbleAnchorHelper::BubbleAnchorHelper(views::BubbleDialogDelegateView* bubble,
+                                       LocationBarDecoration* decoration)
+    : observer_tokens_([[NSMutableArray alloc] init]),
+      bubble_(bubble),
+      decoration_(decoration) {
   DCHECK(bubble->GetWidget());
   DCHECK(bubble->parent_window());
   bubble->GetWidget()->AddObserver(this);
 
+  if (decoration_)
+    decoration_->SetActive(true);
+
   NSRect parent_frame = [[bubble->parent_window() window] frame];
   NSRect bubble_frame = [bubble->GetWidget()->GetNativeWindow() frame];
 
   // Note: when anchored on the right, this doesn't support changes to the
   // bubble size, just the parent size.
   horizontal_offset_ =
       (IsMinXFixed() ? NSMinX(parent_frame) : NSMaxX(parent_frame)) -
       NSMinX(bubble_frame);
   vertical_offset_ = NSMaxY(parent_frame) - NSMinY(bubble_frame);
 
@@ skipping 27 matching lines @@
     bubble_frame.origin.x = NSMinX(parent_frame) - horizontal_offset_;
   else
     bubble_frame.origin.x = NSMaxX(parent_frame) - horizontal_offset_;
   bubble_frame.origin.y = NSMaxY(parent_frame) - vertical_offset_;
   [bubble_->GetWidget()->GetNativeWindow() setFrame:bubble_frame
                                             display:YES
                                             animate:NO];
 }
 
 void BubbleAnchorHelper::OnWidgetDestroying(views::Widget* widget) {
+  if (decoration_)

[tapted, 2017/05/16 07:39:37]

This raises a new lifetime issue that didn't previously exist -
BubbleAnchorHelper::OnWidgetDestroying needs a guarantee that |decoration_|
hasn't been deleted.

I *think* WidgetOwnerNSWindowAdapter provides this guarantee, but we should put
an ownership comment up around line 50. E.g. 

// Weak. Lifetime is tied to the controller of bubble's parent window, which
owns a LocationBarViewMac that directly owns |decoration_|.

And here, a comment like,

// NativeWidgetMac guarantees that a child's OnWidgetDestroying() is invoked
before the parent NSWindow has closed.

[spqchan, 2017/05/17 01:39:39]

Done. Are there any other steps that we could perhaps take to make sure this is
guaranteed?

+    decoration_->SetActive(false);
   widget->RemoveObserver(this);
   for (id token in observer_tokens_.get())
     [[NSNotificationCenter defaultCenter] removeObserver:token];
   delete this;
 }