Change the way the NonClientView handles forcing the native frame for popups/app windows.

Rather than carrying state in a force_native_frame_ member it uses a virtual method AlwaysUseNativeFrame analogous to AlwaysUseCustomFrame. This makes me a little happier.

BUG=none
TEST=On a vista capable system, test that when a theme is installed, popups and app frames are rendered with the native frame. Test that constrained windows (e.g. HTTP basic auth) are rendered with the custom frame. Without a theme installed, test that all windows have a native frame except constrained windows. With Vista Basic system setting, verify that all windows have a custom frame, including constrained windows.

Review URL: http://codereview.chromium.org/200146

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

11 files changed, 82 insertions, 64 deletions

diff --git a/chrome/browser/views/frame/browser_frame.h b/chrome/browser/views/frame/browser_frame.h
index ca8ae7c..fd03d41 100644
--- a/chrome/browser/views/frame/browser_frame.h
+++ b/chrome/browser/views/frame/browser_frame.h
@@ -58,6 +58,11 @@ class BrowserFrame {
 
   // Returns the theme provider for this frame.
   virtual ThemeProvider* GetThemeProviderForFrame() const = 0;
+
+  // Returns true if the window should use the native frame view. This is true
+  // if there are no themes applied on Vista, or if there are themes applied and
+  // this browser window is an app or popup.
+  virtual bool AlwaysUseNativeFrame() const = 0;
 };
 
 #endif  // CHROME_BROWSER_VIEWS_FRAME_BROWSER_FRAME_H_
diff --git a/chrome/browser/views/frame/browser_frame_gtk.cc b/chrome/browser/views/frame/browser_frame_gtk.cc
index 2e7b734..c977c74 100644
--- a/chrome/browser/views/frame/browser_frame_gtk.cc
+++ b/chrome/browser/views/frame/browser_frame_gtk.cc
@@ -69,6 +69,10 @@ ThemeProvider* BrowserFrameGtk::GetThemeProviderForFrame() const {
   return GetThemeProvider();
 }
 
+bool BrowserFrameGtk::AlwaysUseNativeFrame() const {
+  return false;
+}
+
 ThemeProvider* BrowserFrameGtk::GetThemeProvider() const {
   return profile_->GetThemeProvider();
 }
@@ -88,4 +92,3 @@ void BrowserFrameGtk::IsActiveChanged() {
   views::WindowGtk::IsActiveChanged();
 }
 
-
diff --git a/chrome/browser/views/frame/browser_frame_gtk.h b/chrome/browser/views/frame/browser_frame_gtk.h
index 75ec308..5b8a561 100644
--- a/chrome/browser/views/frame/browser_frame_gtk.h
+++ b/chrome/browser/views/frame/browser_frame_gtk.h
@@ -24,7 +24,7 @@ class BrowserFrameGtk : public BrowserFrame,
   // separate to avoid recursive calling of the frame from its constructor.
   void Init();
 
-  // BrowserFrame implementation.
+  // Overridden from BrowserFrame:
   virtual views::Window* GetWindow();
   virtual void TabStripCreated(TabStripWrapper* tabstrip);
   virtual int GetMinimizeButtonOffset() const;
@@ -32,14 +32,15 @@ class BrowserFrameGtk : public BrowserFrame,
   virtual void UpdateThrobber(bool running);
   virtual void ContinueDraggingDetachedTab();
   virtual ThemeProvider* GetThemeProviderForFrame() const;
+  virtual bool AlwaysUseNativeFrame() const;
 
-  // Overridden from views::Widget.
+  // Overridden from views::Widget:
   virtual ThemeProvider* GetThemeProvider() const;
   virtual ThemeProvider* GetDefaultThemeProvider() const;
   virtual void IsActiveChanged();
 
  protected:
-  // WidgetGtk overrides.
+  // Overridden from views::WidgetGtk:
   virtual views::RootView* CreateRootView();
 
  private:
diff --git a/chrome/browser/views/frame/browser_frame_win.cc b/chrome/browser/views/frame/browser_frame_win.cc
index 80cd7b2..6da09b5 100644
--- a/chrome/browser/views/frame/browser_frame_win.cc
+++ b/chrome/browser/views/frame/browser_frame_win.cc
@@ -52,10 +52,6 @@ BrowserFrameWin::BrowserFrameWin(BrowserView* browser_view, Profile* profile)
   GetNonClientView()->SetFrameView(CreateFrameViewForWindow());
   // Don't focus anything on creation, selecting a tab will set the focus.
   set_focus_on_creation(false);
-  // Force popups and apps under Vista to have a nonthemed frame.
-  if (win_util::ShouldUseVistaFrame() &&
-      !browser_view_->IsBrowserTypeNormal())
-    GetNonClientView()->ForceAeroGlassFrame();
 }
 
 void BrowserFrameWin::Init() {
@@ -109,12 +105,15 @@ ThemeProvider* BrowserFrameWin::GetThemeProviderForFrame() const {
   return GetThemeProvider();
 }
 
-ThemeProvider* BrowserFrameWin::GetThemeProvider() const {
-  return profile_->GetThemeProvider();
-}
-
-ThemeProvider* BrowserFrameWin::GetDefaultThemeProvider() const {
-  return profile_->GetThemeProvider();
+bool BrowserFrameWin::AlwaysUseNativeFrame() const {
+  // We use the native frame when we're told we should by the theme provider
+  // (e.g. no custom theme is active), or when we're a popup or app window. We
+  // don't theme popup or app windows, so regardless of whether or not a theme
+  // is active for normal browser windows, we don't want to use the custom frame
+  // for popups/apps.
+  return GetThemeProvider()->ShouldUseNativeFrame() ||
+      (!browser_view_->IsBrowserTypeNormal() &&
+      win_util::ShouldUseVistaFrame());
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -296,6 +295,14 @@ void BrowserFrameWin::OnWindowPosChanged(WINDOWPOS* window_pos) {
   WindowWin::OnWindowPosChanged(window_pos);
 }
 
+ThemeProvider* BrowserFrameWin::GetThemeProvider() const {
+  return profile_->GetThemeProvider();
+}
+
+ThemeProvider* BrowserFrameWin::GetDefaultThemeProvider() const {
+  return profile_->GetThemeProvider();
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserFrame, views::CustomFrameWindow overrides:
 
@@ -304,9 +311,7 @@ int BrowserFrameWin::GetShowState() const {
 }
 
 views::NonClientFrameView* BrowserFrameWin::CreateFrameViewForWindow() {
-  if (GetThemeProvider()->ShouldUseNativeFrame() ||
-      (!browser_view_->IsBrowserTypeNormal() &&
-      win_util::ShouldUseVistaFrame()))
+  if (AlwaysUseNativeFrame())
     browser_frame_view_ = new GlassBrowserFrameView(this, browser_view_);
   else
     browser_frame_view_ = new OpaqueBrowserFrameView(this, browser_view_);
@@ -327,31 +332,28 @@ views::RootView* BrowserFrameWin::CreateRootView() {
 // BrowserFrame, private:
 
 void BrowserFrameWin::UpdateDWMFrame() {
-  // Nothing to do yet.
-  if (!GetClientView() || !browser_view_->IsBrowserTypeNormal() ||
-      !win_util::ShouldUseVistaFrame())
+  // Nothing to do yet, or we're not showing a DWM frame.
+  if (!GetClientView() || !AlwaysUseNativeFrame())
     return;
 
-  // In fullscreen mode, we don't extend glass into the client area at all,
-  // because the GDI-drawn text in the web content composited over it will
-  // become semi-transparent over any glass area.
   MARGINS margins = { 0 };
-  if (!IsMaximized() && !IsFullscreen()) {
-    margins.cxLeftWidth = kClientEdgeThickness + 1;
-    margins.cxRightWidth = kClientEdgeThickness + 1;
-    margins.cyBottomHeight = kClientEdgeThickness + 1;
-  }
-  // In maximized mode, we only have a titlebar strip of glass, no side/bottom
-  // borders.
-  if (!browser_view_->IsFullscreen()) {
-    margins.cyTopHeight =
-        GetBoundsForTabStrip(browser_view_->tabstrip()).bottom();
-  }
-
-  // If DWM is supported, we may still not want to use the DWM frame if we're in
-  // opaque mode (e.g. showing a theme). In this case we want to reset the DWM
-  // frame extending.
-  if (!GetNonClientView()->UseNativeFrame()) {
+  if (browser_view_->IsBrowserTypeNormal()) {
+    // In fullscreen mode, we don't extend glass into the client area at all,
+    // because the GDI-drawn text in the web content composited over it will
+    // become semi-transparent over any glass area.
+    if (!IsMaximized() && !IsFullscreen()) {
+      margins.cxLeftWidth = kClientEdgeThickness + 1;
+      margins.cxRightWidth = kClientEdgeThickness + 1;
+      margins.cyBottomHeight = kClientEdgeThickness + 1;
+    }
+    // In maximized mode, we only have a titlebar strip of glass, no side/bottom
+    // borders.
+    if (!browser_view_->IsFullscreen()) {
+      margins.cyTopHeight =
+          GetBoundsForTabStrip(browser_view_->tabstrip()).bottom();
+    }
+  } else {
+    // For popup and app windows we want to use the default margins.
     margins.cxLeftWidth = margins.cxRightWidth = margins.cyTopHeight =
         margins.cyBottomHeight = 0;
   }
diff --git a/chrome/browser/views/frame/browser_frame_win.h b/chrome/browser/views/frame/browser_frame_win.h
index a4f7fe4..8d8033f 100644
--- a/chrome/browser/views/frame/browser_frame_win.h
+++ b/chrome/browser/views/frame/browser_frame_win.h
@@ -34,6 +34,8 @@ class BrowserFrameWin : public BrowserFrame, public views::WindowWin {
   // separate to avoid recursive calling of the frame from its constructor.
   void Init();
 
+  BrowserView* browser_view() const { return browser_view_; }
+
   // BrowserFrame implementation.
   virtual views::Window* GetWindow();
   virtual void TabStripCreated(TabStripWrapper* tabstrip);
@@ -42,12 +44,7 @@ class BrowserFrameWin : public BrowserFrame, public views::WindowWin {
   virtual void UpdateThrobber(bool running);
   virtual void ContinueDraggingDetachedTab();
   virtual ThemeProvider* GetThemeProviderForFrame() const;
-
-  // Overridden from views::Widget.
-  virtual ThemeProvider* GetThemeProvider() const;
-  virtual ThemeProvider* GetDefaultThemeProvider() const;
-
-  BrowserView* browser_view() const { return browser_view_; }
+  virtual bool AlwaysUseNativeFrame() const;
 
  protected:
   // Overridden from views::WidgetWin:
@@ -66,6 +63,8 @@ class BrowserFrameWin : public BrowserFrame, public views::WindowWin {
   virtual LRESULT OnNCCalcSize(BOOL mode, LPARAM l_param);
   virtual LRESULT OnNCHitTest(const CPoint& pt);
   virtual void OnWindowPosChanged(WINDOWPOS* window_pos);
+  virtual ThemeProvider* GetThemeProvider() const;
+  virtual ThemeProvider* GetDefaultThemeProvider() const;
 
   // Overridden from views::Window:
   virtual int GetShowState() const;
diff --git a/chrome/browser/views/frame/glass_browser_frame_view.cc b/chrome/browser/views/frame/glass_browser_frame_view.cc
index d7c8139..a765416 100644
--- a/chrome/browser/views/frame/glass_browser_frame_view.cc
+++ b/chrome/browser/views/frame/glass_browser_frame_view.cc
@@ -118,6 +118,10 @@ gfx::Rect GlassBrowserFrameView::GetBoundsForClientView() const {
   return client_view_bounds_;
 }
 
+bool GlassBrowserFrameView::AlwaysUseNativeFrame() const {
+  return frame_->AlwaysUseNativeFrame();
+}
+
 gfx::Rect GlassBrowserFrameView::GetWindowBoundsForClientBounds(
     const gfx::Rect& client_bounds) const {
   HWND hwnd = frame_->GetWindow()->GetNativeWindow();
diff --git a/chrome/browser/views/frame/glass_browser_frame_view.h b/chrome/browser/views/frame/glass_browser_frame_view.h
index 81e5000..a686f09 100644
--- a/chrome/browser/views/frame/glass_browser_frame_view.h
+++ b/chrome/browser/views/frame/glass_browser_frame_view.h
@@ -25,6 +25,7 @@ class GlassBrowserFrameView : public BrowserNonClientFrameView {
 
   // Overridden from views::NonClientFrameView:
   virtual gfx::Rect GetBoundsForClientView() const;
+  virtual bool AlwaysUseNativeFrame() const;
   virtual gfx::Rect GetWindowBoundsForClientBounds(
       const gfx::Rect& client_bounds) const;
   virtual gfx::Point GetSystemMenuPoint() const;
diff --git a/chrome/browser/views/frame/opaque_browser_frame_view.cc b/chrome/browser/views/frame/opaque_browser_frame_view.cc
index be39c8d..5ebeb71 100644
--- a/chrome/browser/views/frame/opaque_browser_frame_view.cc
+++ b/chrome/browser/views/frame/opaque_browser_frame_view.cc
@@ -252,6 +252,10 @@ gfx::Rect OpaqueBrowserFrameView::GetBoundsForClientView() const {
   return client_view_bounds_;
 }
 
+bool OpaqueBrowserFrameView::AlwaysUseNativeFrame() const {
+  return frame_->AlwaysUseNativeFrame();
+}
+
 gfx::Rect OpaqueBrowserFrameView::GetWindowBoundsForClientBounds(
     const gfx::Rect& client_bounds) const {
   int top_height = NonClientTopBorderHeight();
diff --git a/chrome/browser/views/frame/opaque_browser_frame_view.h b/chrome/browser/views/frame/opaque_browser_frame_view.h
index 81d2b36..720b2be5 100644
--- a/chrome/browser/views/frame/opaque_browser_frame_view.h
+++ b/chrome/browser/views/frame/opaque_browser_frame_view.h
@@ -39,6 +39,7 @@ class OpaqueBrowserFrameView : public BrowserNonClientFrameView,
  protected:
   // Overridden from views::NonClientFrameView:
   virtual gfx::Rect GetBoundsForClientView() const;
+  virtual bool AlwaysUseNativeFrame() const;
   virtual gfx::Rect GetWindowBoundsForClientBounds(
       const gfx::Rect& client_bounds) const;
   virtual gfx::Point GetSystemMenuPoint() const;
diff --git a/views/window/non_client_view.cc b/views/window/non_client_view.cc
index a97531f..29c48cc 100644
--- a/views/window/non_client_view.cc
+++ b/views/window/non_client_view.cc
@@ -32,8 +32,7 @@ static const int kClientViewIndex = 1;
 
 NonClientView::NonClientView(Window* frame)
     : frame_(frame),
-      client_view_(NULL),
-      force_aero_glass_frame_(false) {
+      client_view_(NULL) {
 }
 
 NonClientView::~NonClientView() {
@@ -69,11 +68,17 @@ void NonClientView::UpdateFrame() {
 }
 
 bool NonClientView::UseNativeFrame() const {
-  if (force_aero_glass_frame_)
-    return true;
-  // The frame view may always require a custom frame, e.g. Constrained Windows.
-  if (frame_view_.get() && frame_view_->AlwaysUseCustomFrame())
-    return false;
+  if (frame_view_.get()) {
+    // The frame view may always require a native frame, e.g. popups on Vista+
+    // when themes are active.
+    if (frame_view_->AlwaysUseNativeFrame())
+      return true;
+
+    // The frame view may always require a custom frame, e.g. Constrained
+    // Windows.
+    if (frame_view_->AlwaysUseCustomFrame())
+      return false;
+  }
   return frame_->ShouldUseNativeFrame();
 }
 
@@ -204,10 +209,6 @@ void NonClientView::SetAccessibleName(const std::wstring& name) {
   accessible_name_ = name;
 }
 
-void NonClientView::ForceAeroGlassFrame() {
-  force_aero_glass_frame_ = true;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // NonClientFrameView, View overrides:
 
diff --git a/views/window/non_client_view.h b/views/window/non_client_view.h
index c4ae4b0..6ed0dcf 100644
--- a/views/window/non_client_view.h
+++ b/views/window/non_client_view.h
@@ -46,6 +46,11 @@ class NonClientFrameView : public View {
   // which is a child window and must always provide its own frame.
   virtual bool AlwaysUseCustomFrame() const { return false; }
 
+  // Like AlwaysUseCustomFrame, returns true if this FrameView should always use
+  // the native frame, regardless of theme settings. An example is popup/app
+  // windows, which we do not ever want to show themed.
+  virtual bool AlwaysUseNativeFrame() const { return false; }
+
   virtual gfx::Rect GetWindowBoundsForClientBounds(
       const gfx::Rect& client_bounds) const = 0;
   virtual gfx::Point GetSystemMenuPoint() const = 0;
@@ -198,10 +203,6 @@ class NonClientView : public View {
   virtual bool GetAccessibleName(std::wstring* name);
   virtual void SetAccessibleName(const std::wstring& name);
 
-  // Call if the nonclientview is in an app or popup and we are in Vista, to
-  // force usage of glass frame.
-  void ForceAeroGlassFrame();
-
  protected:
   // NonClientView, View overrides:
   virtual void ViewHierarchyChanged(bool is_add, View* parent, View* child);
@@ -224,10 +225,6 @@ class NonClientView : public View {
   // The accessible name of this view.
   std::wstring accessible_name_;
 
-  // True if the nonclientview is in an app or popup and we are in Vista. Used
-  // to force usage of glass frame.
-  bool force_aero_glass_frame_;
-
   DISALLOW_COPY_AND_ASSIGN(NonClientView);
 };