Make Web Intents picker in Views conform to latest mocks

chrome/browser/ui/views/constrained_window_views.cc

 // 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.
 
 #include "chrome/browser/ui/views/constrained_window_views.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/constrained_window_tab_helper.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/browser/ui/toolbar/toolbar_model.h"
 #include "chrome/browser/ui/views/constrained_window_frame_simple.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
+#include "content/public/browser/navigation_controller.h"

[Peter Kasting, 2012/10/15 21:46:54]

Nit: Alphabetical order

[please use gerrit instead, 2012/10/16 00:12:23]

Done.

+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_source.h"
+#include "content/public/browser/notification_types.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "grit/ui_resources.h"
 #include "net/base/net_util.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/base/hit_test.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/font.h"
@@ skipping 538 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(ConstrainedWindowFrameViewAsh);
 };
 #endif  // defined(USE_ASH)
 
 ////////////////////////////////////////////////////////////////////////////////
 // ConstrainedWindowViews, public:
 
 ConstrainedWindowViews::ConstrainedWindowViews(
     content::WebContents* web_contents,
     views::WidgetDelegate* widget_delegate,
-    bool enable_chrome_style)
+    bool enable_chrome_style,
+    const gfx::Insets& client_insets)
     : WebContentsObserver(web_contents),
       web_contents_(web_contents),
+      enable_chrome_style_(enable_chrome_style),
+      client_insets_(client_insets),
       ALLOW_THIS_IN_INITIALIZER_LIST(native_constrained_window_(
-          NativeConstrainedWindow::CreateNativeConstrainedWindow(this))),
-      enable_chrome_style_(enable_chrome_style) {
+          NativeConstrainedWindow::CreateNativeConstrainedWindow(this))) {
   views::Widget::InitParams params(views::Widget::InitParams::TYPE_WINDOW);
   params.delegate = widget_delegate;
   params.native_widget = native_constrained_window_->AsNativeWidget();
   params.child = true;
 
   if (enable_chrome_style_) {
     params.parent_widget = Widget::GetTopLevelWidgetForNativeView(
-        web_contents->GetView()->GetNativeView());
+        web_contents_->GetView()->GetNativeView());
   } else {
-    params.parent = web_contents->GetNativeView();
+    params.parent = web_contents_->GetNativeView();
   }
 
 #if defined(USE_ASH)
   // Ash window headers can be transparent.
   params.transparent = true;
   ash::SetChildWindowVisibilityChangesAnimated(params.GetParent());
   // No animations should get performed on the window since that will re-order
   // the window stack which will then cause many problems.
   if (params.parent && params.parent->parent()) {
     params.parent->parent()->SetProperty(aura::client::kAnimationsDisabledKey,
                                          true);
   }
 #endif
   Init(params);
 
   if (enable_chrome_style_) {
     // Set the dialog background color.
     if (widget_delegate && widget_delegate->AsDialogDelegate()) {
       views::Background* background = views::Background::CreateSolidBackground(
           ConstrainedWindow::GetBackgroundColor());
       views::DialogClientView* dialog_client_view =
           widget_delegate->AsDialogDelegate()->GetDialogClientView();
       if (dialog_client_view)
         dialog_client_view->set_background(background);
     }
-    PositionChromeStyleWindow();
+    PositionChromeStyleWindow(GetRootView()->bounds().size());
+    registrar_.Add(this,
+                   content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED,
+                   content::Source<content::WebContents>(web_contents));
   }
 
   ConstrainedWindowTabHelper* constrained_window_tab_helper =
       ConstrainedWindowTabHelper::FromWebContents(web_contents_);
   constrained_window_tab_helper->AddConstrainedDialog(this);
 #if defined(USE_ASH)
   GetNativeWindow()->SetProperty(ash::kConstrainedWindowKey, true);
 #endif
 }
 
@@ skipping 45 matching lines @@
 
   ConstrainedWindowTabHelper* constrained_window_tab_helper =
       ConstrainedWindowTabHelper::FromWebContents(web_contents_);
   constrained_window_tab_helper->WillClose(this);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ConstrainedWindowViews, views::Widget overrides:
 
 void ConstrainedWindowViews::CenterWindow(const gfx::Size& size) {
-  Widget::CenterWindow(size);
   if (enable_chrome_style_)
-    PositionChromeStyleWindow();
+    PositionChromeStyleWindow(size);
+  else
+    Widget::CenterWindow(size);
 }
 
 views::NonClientFrameView* ConstrainedWindowViews::CreateNonClientFrameView() {
   if (enable_chrome_style_) {
-    return new ConstrainedWindowFrameSimple(this);
+    ConstrainedWindowFrameSimple* frame =
+        new ConstrainedWindowFrameSimple(this);
+    frame->set_border(views::Border::CreateEmptyBorder(
+        client_insets_.top(),
+        client_insets_.left(),
+        client_insets_.bottom(),
+        client_insets_.right()));
+    return frame;
   } else {

[Peter Kasting, 2012/10/15 21:46:54]

Nit: No else after return

[please use gerrit instead, 2012/10/16 00:12:23]

Done.

 #if defined(USE_ASH)
     CommandLine* command_line = CommandLine::ForCurrentProcess();
     if (command_line->HasSwitch(ash::switches::kAuraGoogleDialogFrames))
       return ash::Shell::GetInstance()->CreateDefaultNonClientFrameView(this);
     ConstrainedWindowFrameViewAsh* frame = new ConstrainedWindowFrameViewAsh;
     frame->Init(this);
     return frame;
 #endif
     return new ConstrainedWindowFrameView(this);
   }
@@ skipping 13 matching lines @@
 views::internal::NativeWidgetDelegate*
     ConstrainedWindowViews::AsNativeWidgetDelegate() {
   return this;
 }
 
 int ConstrainedWindowViews::GetNonClientComponent(const gfx::Point& point) {
   // Prevent a constrained window to be moved by the user.
   return HTNOWHERE;
 }
 
-void ConstrainedWindowViews::PositionChromeStyleWindow() {
+void ConstrainedWindowViews::PositionChromeStyleWindow(const gfx::Size& size) {
   DCHECK(enable_chrome_style_);
-  gfx::Rect bounds = GetRootView()->bounds();
+  gfx::Rect bounds(GetRootView()->bounds());
+  bounds.set_size(size);

[Peter Kasting, 2012/10/15 21:46:54]

Nit: How about:

  gfx::rect bounds(GetRootView()->bounds().origin(), size);

[please use gerrit instead, 2012/10/16 00:12:23]

That's a cool trick, thanks! Done.

   ConstrainedWindowTabHelperDelegate* tab_helper_delegate =
       ConstrainedWindowTabHelper::FromWebContents(web_contents_)->delegate();
-
   BrowserWindow* browser_window =
       tab_helper_delegate ? tab_helper_delegate->GetBrowserWindow() : NULL;
   int top_y;

[Peter Kasting, 2012/10/15 21:46:54]

Nit: Declare this inside the conditional

[please use gerrit instead, 2012/10/16 00:12:23]

Done.

-  if (browser_window && browser_window->GetConstrainedWindowTopY(&top_y)) {
-    bounds.set_y(top_y);
-    bounds.set_x(
-        browser_window->GetBounds().width() / 2 - bounds.width() / 2);
-    SetBounds(bounds);
+  if (browser_window) {
+    bounds.set_x(browser_window->GetBounds().width() / 2 - bounds.width() / 2);
+    if (browser_window->GetConstrainedWindowTopY(&top_y))
+      bounds.set_y(top_y);
   }
+  SetBounds(bounds);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ConstrainedWindowViews, content::WebContentsObserver implementation:
 
 void ConstrainedWindowViews::WebContentsDestroyed(
     content::WebContents* web_contents) {
   web_contents_ = NULL;
 }
+
+////////////////////////////////////////////////////////////////////////////////
+// ConstrainedWindowViews, content::NotificationObserver implementation:

[Peter Kasting, 2012/10/15 21:46:54]

Nit: This practice of separating each group of overrides with its own section
header is fairly unusual... I'd remove all of these

[please use gerrit instead, 2012/10/16 00:12:23]

Removed all //////////////. Done.

+
+void ConstrainedWindowViews::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  if (enable_chrome_style_ &&
+      type == content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED) {

[Peter Kasting, 2012/10/15 21:46:54]

Nit: DCHECK_EQ that |type| is this value atop the function

[please use gerrit instead, 2012/10/16 00:12:23]

I've added DCHECK(enable_chrome_style_) and DHCECK_EQ(type, ...) on top and
removed the if-statement below. Done.

+    if (*content::Details<bool>(details).ptr())
+      Show();
+    else
+      Hide();
+  }
+}