Ash: Correctly initialize transparency flag for windows

Widgets of TYPE_WINDOW have potentially transparent frames on Ash.  I was initializing the transparency flag in the wrong place, leading to a subset of dialogs (edit bookmark, app shortcut, restart chrome, etc.) having black pixels at their outside corners and the wrong appearance in the inactive state.  Inspection of the Widget::CreateWindow* callers suggests this is the right place to put this.

BUG=117411
TEST=visual inspection of bookmark windows

Review URL: https://chromiumcodereview.appspot.com/9653011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@125894 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 81 insertions, 33 deletions

diff --git a/ash/shell/shell_main.cc b/ash/shell/shell_main.cc
index c1c6d83..339010b 100644
--- a/ash/shell/shell_main.cc
+++ b/ash/shell/shell_main.cc
@@ -45,6 +45,10 @@ class ShellViewsDelegate : public views::TestViewsDelegate {
       views::Widget* widget) OVERRIDE {
     return ash::Shell::GetInstance()->CreateDefaultNonClientFrameView(widget);
   }
+  bool UseTransparentWindows() const OVERRIDE {
+    // Ash uses transparent window frames.
+    return true;
+  }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ShellViewsDelegate);
diff --git a/chrome/browser/ui/views/accessibility_event_router_views_unittest.cc b/chrome/browser/ui/views/accessibility_event_router_views_unittest.cc
index 481f9c3..24acc13 100644
--- a/chrome/browser/ui/views/accessibility_event_router_views_unittest.cc
+++ b/chrome/browser/ui/views/accessibility_event_router_views_unittest.cc
@@ -68,6 +68,9 @@ class AccessibilityViewsDelegate : public views::ViewsDelegate {
       views::Widget* widget) OVERRIDE {
     return NULL;
   }
+  virtual bool UseTransparentWindows() const OVERRIDE {
+    return false;
+  }
   virtual void AddRef() OVERRIDE {}
   virtual void ReleaseRef() OVERRIDE {}
 
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_bar_view_test.cc b/chrome/browser/ui/views/bookmarks/bookmark_bar_view_test.cc
index 218d83a4..cc7a60f 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_bar_view_test.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_bar_view_test.cc
@@ -105,6 +105,10 @@ class ViewsDelegateImpl : public views::ViewsDelegate {
     return NULL;
   }
 
+  virtual bool UseTransparentWindows() const OVERRIDE {
+    return false;
+  }
+
   virtual void AddRef() OVERRIDE {
   }
 
diff --git a/chrome/browser/ui/views/chrome_views_delegate.cc b/chrome/browser/ui/views/chrome_views_delegate.cc
index f5aceb5..bf99c47 100644
--- a/chrome/browser/ui/views/chrome_views_delegate.cc
+++ b/chrome/browser/ui/views/chrome_views_delegate.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -24,7 +24,7 @@
 #include "chrome/browser/app_icon_win.h"
 #endif
 
-#if defined(USE_AURA)
+#if defined(USE_ASH)
 #include "ash/shell.h"
 #endif
 
@@ -130,13 +130,22 @@ HICON ChromeViewsDelegate::GetDefaultWindowIcon() const {
 
 views::NonClientFrameView* ChromeViewsDelegate::CreateDefaultNonClientFrameView(
     views::Widget* widget) {
-#if defined(USE_AURA)
+#if defined(USE_ASH)
   return ash::Shell::GetInstance()->CreateDefaultNonClientFrameView(widget);
 #else
   return NULL;
 #endif
 }
 
+bool ChromeViewsDelegate::UseTransparentWindows() const {
+#if defined(USE_ASH)
+  // Ash uses transparent window frames above.
+  return true;
+#else
+  return false;
+#endif
+}
+
 void ChromeViewsDelegate::AddRef() {
   g_browser_process->AddRefModule();
 }
diff --git a/chrome/browser/ui/views/chrome_views_delegate.h b/chrome/browser/ui/views/chrome_views_delegate.h
index 136b5e8..315f7ab 100644
--- a/chrome/browser/ui/views/chrome_views_delegate.h
+++ b/chrome/browser/ui/views/chrome_views_delegate.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -40,6 +40,7 @@ class ChromeViewsDelegate : public views::ViewsDelegate {
 #endif
   virtual views::NonClientFrameView* CreateDefaultNonClientFrameView(
       views::Widget* widget) OVERRIDE;
+  virtual bool UseTransparentWindows() const OVERRIDE;
   virtual void AddRef() OVERRIDE;
   virtual void ReleaseRef() OVERRIDE;
 
diff --git a/chrome/browser/ui/views/menu_model_adapter_test.cc b/chrome/browser/ui/views/menu_model_adapter_test.cc
index 2966bab..10851fad 100644
--- a/chrome/browser/ui/views/menu_model_adapter_test.cc
+++ b/chrome/browser/ui/views/menu_model_adapter_test.cc
@@ -72,6 +72,10 @@ class TestViewsDelegate : public views::ViewsDelegate {
     return NULL;
   }
 
+  virtual bool UseTransparentWindows() const OVERRIDE {
+    return false;
+  }
+
   virtual void AddRef() OVERRIDE {
   }
 
diff --git a/chrome/browser/ui/views/window.cc b/chrome/browser/ui/views/window.cc
index c55594a..cb6b463 100644
--- a/chrome/browser/ui/views/window.cc
+++ b/chrome/browser/ui/views/window.cc
@@ -35,27 +35,6 @@
 //       The remainder of the code here is dealing with the legacy CrOS WM and
 //       can also be removed.
 
-namespace {
-
-views::Widget* CreateViewsWindowWithParent(gfx::NativeWindow parent,
-                                           views::WidgetDelegate* delegate) {
-  views::Widget* widget = new views::Widget;
-  views::Widget::InitParams params;
-  params.delegate = delegate;
-#if defined(OS_WIN) || defined(USE_AURA)
-  params.parent = parent;
-#endif
-#if defined(USE_AURA)
-  // Aura dialogs may have translucent frames.
-  // TODO(jamescook): Find a better way to set this.
-  params.transparent = true;
-#endif
-  widget->Init(params);
-  return widget;
-}
-
-}  // namespace
-
 namespace browser {
 
 views::Widget* CreateViewsWindow(gfx::NativeWindow parent,
@@ -64,7 +43,7 @@ views::Widget* CreateViewsWindow(gfx::NativeWindow parent,
 #if defined(OS_CHROMEOS) && !defined(USE_AURA)
   return chromeos::BubbleWindow::Create(parent, style, delegate);
 #else
-  return CreateViewsWindowWithParent(parent, delegate);
+  return views::Widget::CreateWindowWithParent(delegate, parent);
 #endif
 }
 
@@ -90,6 +69,7 @@ views::Widget* CreateFramelessWindowWithParentAndBounds(
 #if defined(OS_WIN) || defined(USE_AURA)
   params.parent = parent;
 #endif
+  // No frame so does not need params.transparent = true
   params.bounds = bounds;
   widget->Init(params);
   return widget;
diff --git a/ui/views/test/test_views_delegate.cc b/ui/views/test/test_views_delegate.cc
index 73eef06..6fa6bb7 100644
--- a/ui/views/test/test_views_delegate.cc
+++ b/ui/views/test/test_views_delegate.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -9,7 +9,8 @@
 
 namespace views {
 
-TestViewsDelegate::TestViewsDelegate() {
+TestViewsDelegate::TestViewsDelegate()
+    : use_transparent_windows_(false) {
   DCHECK(!ViewsDelegate::views_delegate);
   ViewsDelegate::views_delegate = this;
 }
@@ -18,6 +19,10 @@ TestViewsDelegate::~TestViewsDelegate() {
   ViewsDelegate::views_delegate = NULL;
 }
 
+void TestViewsDelegate::SetUseTransparentWindows(bool transparent) {
+  use_transparent_windows_ = transparent;
+}
+
 ui::Clipboard* TestViewsDelegate::GetClipboard() const {
   if (!clipboard_.get()) {
     // Note that we need a MessageLoop for the next call to work.
@@ -44,6 +49,10 @@ NonClientFrameView* TestViewsDelegate::CreateDefaultNonClientFrameView(
   return NULL;
 }
 
+bool TestViewsDelegate::UseTransparentWindows() const {
+  return use_transparent_windows_;
+}
+
 int TestViewsDelegate::GetDispositionForEvent(int event_flags) {
   return 0;
 }
diff --git a/ui/views/test/test_views_delegate.h b/ui/views/test/test_views_delegate.h
index 5fa952d..99976a2 100644
--- a/ui/views/test/test_views_delegate.h
+++ b/ui/views/test/test_views_delegate.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -25,6 +25,8 @@ class TestViewsDelegate : public ViewsDelegate {
   TestViewsDelegate();
   virtual ~TestViewsDelegate();
 
+  void SetUseTransparentWindows(bool transparent);
+
   // Overridden from ViewsDelegate:
   virtual ui::Clipboard* GetClipboard() const OVERRIDE;
   virtual void SaveWindowPlacement(const Widget* window,
@@ -51,6 +53,7 @@ class TestViewsDelegate : public ViewsDelegate {
 #endif
   virtual NonClientFrameView* CreateDefaultNonClientFrameView(
       Widget* widget) OVERRIDE;
+  virtual bool UseTransparentWindows() const OVERRIDE;
   virtual void AddRef() OVERRIDE {}
   virtual void ReleaseRef() OVERRIDE {}
 
@@ -58,6 +61,7 @@ class TestViewsDelegate : public ViewsDelegate {
 
  private:
   mutable scoped_ptr<ui::Clipboard> clipboard_;
+  bool use_transparent_windows_;
 
   DISALLOW_COPY_AND_ASSIGN(TestViewsDelegate);
 };
diff --git a/ui/views/views_delegate.h b/ui/views/views_delegate.h
index 2544413..af9c665 100644
--- a/ui/views/views_delegate.h
+++ b/ui/views/views_delegate.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -84,6 +84,11 @@ class VIEWS_EXPORT ViewsDelegate {
   virtual NonClientFrameView* CreateDefaultNonClientFrameView(
       Widget* widget) = 0;
 
+  // Returns whether the embedding app wants windows to be created with the
+  // views::Widget marked as transparent.  For example, an app may wish to
+  // apply transparent window frames in the NonClientFrameView.
+  virtual bool UseTransparentWindows() const = 0;
+
   // AddRef/ReleaseRef are invoked while a menu is visible. They are used to
   // ensure we don't attempt to exit while a menu is showing.
   virtual void AddRef() = 0;
diff --git a/ui/views/widget/widget.cc b/ui/views/widget/widget.cc
index 569fd92..33ccda3d 100644
--- a/ui/views/widget/widget.cc
+++ b/ui/views/widget/widget.cc
@@ -113,7 +113,8 @@ Widget::InitParams::InitParams()
       delegate(NULL),
       child(false),
       transient(false),
-      transparent(false),
+      transparent(ViewsDelegate::views_delegate &&
+                  ViewsDelegate::views_delegate->UseTransparentWindows()),
       accept_events(true),
       can_activate(true),
       keep_on_top(false),
@@ -135,7 +136,9 @@ Widget::InitParams::InitParams(Type type)
       delegate(NULL),
       child(type == TYPE_CONTROL),
       transient(type == TYPE_BUBBLE || type == TYPE_POPUP || type == TYPE_MENU),
-      transparent(false),
+      transparent(type == TYPE_WINDOW &&
+                  ViewsDelegate::views_delegate &&
+                  ViewsDelegate::views_delegate->UseTransparentWindows()),
       accept_events(true),
       can_activate(
           type != TYPE_POPUP && type != TYPE_MENU && type != TYPE_CONTROL),
diff --git a/ui/views/widget/widget.h b/ui/views/widget/widget.h
index baf7837..a21302b 100644
--- a/ui/views/widget/widget.h
+++ b/ui/views/widget/widget.h
@@ -155,7 +155,8 @@ class VIEWS_EXPORT Widget : public internal::NativeWidgetDelegate,
     bool transient;
     // If true, the widget may be fully or partially transparent.  If false,
     // we can perform optimizations based on the widget being fully opaque.
-    // Defaults to false.
+    // For window widgets, defaults to ViewsDelegate::UseTransparentWindows().
+    // Defaults to false for non-window widgets.
     bool transparent;
     bool accept_events;
     bool can_activate;
diff --git a/ui/views/widget/widget_unittest.cc b/ui/views/widget/widget_unittest.cc
index 7be09b0..143c51e 100644
--- a/ui/views/widget/widget_unittest.cc
+++ b/ui/views/widget/widget_unittest.cc
@@ -159,6 +159,27 @@ ui::WindowShowState GetWidgetShowState(const Widget* widget) {
                               ui::SHOW_STATE_NORMAL;
 }
 
+TEST_F(WidgetTest, WidgetInitParams) {
+  ASSERT_FALSE(views_delegate().UseTransparentWindows());
+
+  // Widgets are not transparent by default.
+  Widget::InitParams init1;
+  EXPECT_FALSE(init1.transparent);
+
+  // Non-window widgets are not transparent either.
+  Widget::InitParams init2(Widget::InitParams::TYPE_MENU);
+  EXPECT_FALSE(init2.transparent);
+
+  // A ViewsDelegate can set windows transparent by default.
+  views_delegate().SetUseTransparentWindows(true);
+  Widget::InitParams init3;
+  EXPECT_TRUE(init3.transparent);
+
+  // Non-window widgets stay opaque.
+  Widget::InitParams init4(Widget::InitParams::TYPE_MENU);
+  EXPECT_FALSE(init4.transparent);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Widget::GetTopLevelWidget tests.