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

chrome/browser/ui/browser_dialogs.h

 // Copyright (c) 2012 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.
 
 #ifndef CHROME_BROWSER_UI_BROWSER_DIALOGS_H_
 #define CHROME_BROWSER_UI_BROWSER_DIALOGS_H_
 
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/callback.h"
 #include "base/optional.h"
 #include "base/strings/string16.h"
 #include "build/build_config.h"
 #include "chrome/browser/ui/bookmarks/bookmark_editor.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/gfx/native_widget_types.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/arc/intent_helper/arc_navigation_throttle.h"
 #endif  // OS_CHROMEOS
 
+#if defined(OS_MACOSX)
+class LocationBarDecoration;
+#endif
+
 class Browser;
 class ContentSettingBubbleModel;
 class GURL;
 class LoginHandler;
 class Profile;
 
 namespace bookmarks {
 class BookmarkBubbleObserver;
 }
 
@@ skipping 82 matching lines @@
                                         SkColor initial_color);
 
 #if defined(OS_MACOSX)
 
 // Shows a Views page info bubble at the given anchor point.
 void ShowPageInfoBubbleViewsAtPoint(
     const gfx::Point& anchor_point,
     Profile* profile,
     content::WebContents* web_contents,
     const GURL& virtual_url,
-    const security_state::SecurityInfo& security_info);
+    const security_state::SecurityInfo& security_info,
+    LocationBarDecoration* decoration);

[sky, 2017/05/19 14:53:22]

The code in c/b/ui shouldn't know about platform specific classes such as
LocationBarDecoration. Is there some other way to inject the decoration? I'm not
clear on why you need decoration here? Presumably only the mac code is going to
be calling this, so can't the mac code call the function defined in the mac
side? I'm suggesting you leave this functions as is here and in the mac code you
define the function that takes a decoration.

[spqchan, 2017/05/19 17:44:55]

Done.

[tapted, 2017/05/21 08:15:28]

Ooh interesting. I think the fact that we can even pass a LocationBarDecoration*
in to these methods (which only exists in a Cocoa browser window), is an
indication that these declarations are just in the wrong place, and should be
moved under c/b/ui/cocoa.

I toyed around with PatchSet 6 in https://codereview.chromium.org/2899583002/ -
I think that's what we want.

Basically it's move: chrome/browser/ui/{views =>
cocoa}/browser_dialogs_views_mac.cc - then we make a
chrome/browser/ui/cocoa/browser_dialogs_views_mac.h to match, which has the
declarations that are only in use by the cocoa browser and can be deleted when
things shift to mac_views_browser

 
 // Show a Views bookmark bubble at the given point. This occurs when the
 // bookmark star is clicked or "Bookmark This Page..." is selected from a menu
 // or via a key equivalent.
 void ShowBookmarkBubbleViewsAtPoint(const gfx::Point& anchor_point,
                                     gfx::NativeView parent,
                                     bookmarks::BookmarkBubbleObserver* observer,
                                     Browser* browser,
                                     const GURL& url,
-                                    bool newly_bookmarked);
+                                    bool newly_bookmarked,
+                                    LocationBarDecoration* decoration);
 
 // Shows a views zoom bubble at the |anchor_point|. This occurs when the zoom
 // icon is clicked or when a shortcut key is pressed or whenever |web_contents|
 // zoom factor changes. |user_action| is used to determine if the bubble will
 // auto-close.
 void ShowZoomBubbleViewsAtPoint(content::WebContents* web_contents,
                                 const gfx::Point& anchor_point,
                                 bool user_action);
 
 // Closes a views zoom bubble if currently shown.
@@ skipping 45 matching lines @@
 
 #if defined(OS_MACOSX)
 
 // This is a class so that it can be friended from ContentSettingBubbleContents,
 // which allows it to call SetAnchorRect().
 class ContentSettingBubbleViewsBridge {
  public:
   static void Show(gfx::NativeView parent_view,
                    ContentSettingBubbleModel* model,
                    content::WebContents* web_contents,
-                   const gfx::Point& anchor);
+                   const gfx::Point& anchor,
+                   LocationBarDecoration* decoration);
+
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(ContentSettingBubbleViewsBridge);
 };
 
 #endif  // OS_MACOSX
 
 #endif  // TOOLKIT_VIEWS
 
 // Values used in the Dialog.Creation UMA metric. Each value represents a
 // different type of dialog box.
@@ skipping 66 matching lines @@
 using BubbleShowPtr =
     void (*)(content::WebContents*,
              const std::vector<arc::ArcNavigationThrottle::AppInfo>&,
              const IntentPickerResponse&);
 
 BubbleShowPtr ShowIntentPickerBubble();
 
 #endif  // OS_CHROMEOS
 
 #endif  // CHROME_BROWSER_UI_BROWSER_DIALOGS_H_