Misc. cleanup in ExtensionPopup code:

* Use constexpr
* Change override comments to match more common style
* Call superclass methods (more technically correct)
* In OnWidgetActivationChanged(), call superclass method before doing override
  code instead of after; this makes it harder to omit calling the superclass via
  early return
* Use CloseWithReason(), as BubbleDialogDelegateView does
* Shorten code by using conditional instead of switch, and not handling DCHECK
  failure

Bug: none
Change-Id: Iffe3a4f229924fad13fd9b08e431a31308821fbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1531652
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#642376}

chrome/browser/ui/views/extensions/extension_popup.cc

@@ -24,14 +24,6 @@
 #include "ui/wm/core/window_animations.h"
 #endif
 
-// The minimum/maximum dimensions of the popup.
-// The minimum is just a little larger than the size of the button itself.
-// The maximum is an arbitrary number that should be smaller than most screens.
-const int ExtensionPopup::kMinWidth = 25;
-const int ExtensionPopup::kMinHeight = 25;
-const int ExtensionPopup::kMaxWidth = 800;
-const int ExtensionPopup::kMaxHeight = 600;
-
 // static
 void ExtensionPopup::ShowPopup(
     std::unique_ptr<extensions::ExtensionViewHost> host,
@@ -59,12 +51,14 @@ ExtensionPopup::~ExtensionPopup() {
 gfx::Size ExtensionPopup::CalculatePreferredSize() const {
   // Constrain the size to popup min/max.
   gfx::Size sz = views::View::CalculatePreferredSize();
-  sz.set_width(std::max(kMinWidth, std::min(kMaxWidth, sz.width())));
-  sz.set_height(std::max(kMinHeight, std::min(kMaxHeight, sz.height())));
+  sz.SetToMax(gfx::Size(kMinWidth, kMinHeight));
+  sz.SetToMin(gfx::Size(kMaxWidth, kMaxHeight));
   return sz;
 }
 
 void ExtensionPopup::AddedToWidget() {
+  BubbleDialogDelegateView::AddedToWidget();
+
   const int radius =
       GetBubbleFrameView()->bubble_border()->GetBorderCornerRadius();
   const bool contents_has_rounded_corners =
@@ -79,13 +73,14 @@ int ExtensionPopup::GetDialogButtons() const {
 
 void ExtensionPopup::OnWidgetActivationChanged(views::Widget* widget,
                                                bool active) {
+  BubbleDialogDelegateView::OnWidgetActivationChanged(widget, active);
+
   if (active && observe_next_widget_activation_) {
     observe_next_widget_activation_ = false;
     views::Widget* const my_widget = GetWidget();
     if (widget != my_widget)
-      my_widget->Close();
+      my_widget->CloseWithReason(views::Widget::ClosedReason::kLostFocus);
   }
-  BubbleDialogDelegateView::OnWidgetActivationChanged(widget, active);
 }
 
 bool ExtensionPopup::ShouldHaveRoundCorners() const {
@@ -99,21 +94,18 @@ void ExtensionPopup::OnExtensionSizeChanged(ExtensionViewViews* view) {
 void ExtensionPopup::Observe(int type,
                              const content::NotificationSource& source,
                              const content::NotificationDetails& details) {
-  switch (type) {
-    case content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME:
-      DCHECK_EQ(host()->host_contents(),
-                content::Source<content::WebContents>(source).ptr());
-      // Show when the content finishes loading and its width is computed.
-      ShowBubble();
-      break;
-    case extensions::NOTIFICATION_EXTENSION_HOST_VIEW_SHOULD_CLOSE:
-      // If we aren't the host of the popup, then disregard the notification.
-      if (content::Details<extensions::ExtensionHost>(host()) == details)
-        GetWidget()->Close();
-      break;
-    default:
-      NOTREACHED() << "Received unexpected notification";
+  if (type == content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME) {
+    DCHECK_EQ(host()->host_contents(),
+              content::Source<content::WebContents>(source).ptr());
+    // Show when the content finishes loading and its width is computed.
+    ShowBubble();
+    return;
   }
+
+  DCHECK_EQ(extensions::NOTIFICATION_EXTENSION_HOST_VIEW_SHOULD_CLOSE, type);
+  // If we aren't the host of the popup, then disregard the notification.
+  if (content::Details<extensions::ExtensionHost>(host()) == details)
+    GetWidget()->Close();
 }
 
 void ExtensionPopup::OnTabStripModelChanged(

chrome/browser/ui/views/extensions/extension_popup.h

@@ -41,10 +41,12 @@ class ExtensionPopup : public views::BubbleDialogDelegateView,
   };
 
   // The min/max height of popups.
-  static const int kMinWidth;
-  static const int kMinHeight;
-  static const int kMaxWidth;
-  static const int kMaxHeight;
+  // The minimum is just a little larger than the size of the button itself.
+  // The maximum is an arbitrary number and should be smaller than most screens.
+  static constexpr int kMinWidth = 25;
+  static constexpr int kMinHeight = 25;
+  static constexpr int kMaxWidth = 800;
+  static constexpr int kMaxHeight = 600;
 
   // Creates and shows a popup with the given |host| positioned adjacent to
   // |anchor_view|.
@@ -63,28 +65,28 @@ class ExtensionPopup : public views::BubbleDialogDelegateView,
 
   extensions::ExtensionViewHost* host() const { return host_.get(); }
 
-  // views::BubbleDialogDelegateView overrides.
+  // views::BubbleDialogDelegateView:
   gfx::Size CalculatePreferredSize() const override;
   void AddedToWidget() override;
   int GetDialogButtons() const override;
   void OnWidgetActivationChanged(views::Widget* widget, bool active) override;
   bool ShouldHaveRoundCorners() const override;
 
-  // ExtensionViewViews::Container overrides.
+  // ExtensionViewViews::Container:
   void OnExtensionSizeChanged(ExtensionViewViews* view) override;
 
-  // content::NotificationObserver overrides.
+  // content::NotificationObserver:
   void Observe(int type,
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override;
 
-  // TabStripModelObserver overrides.
+  // TabStripModelObserver:
   void OnTabStripModelChanged(
       TabStripModel* tab_strip_model,
       const TabStripModelChange& change,
       const TabStripSelectionChange& selection) override;
 
-  // content::DevToolsAgentHostObserver overrides.
+  // content::DevToolsAgentHostObserver:
   void DevToolsAgentHostAttached(
       content::DevToolsAgentHost* agent_host) override;
   void DevToolsAgentHostDetached(