Linux Aura: Use system title bar is now set by preference, not flag.

chrome/browser/ui/views/frame/browser_frame.cc

Index: chrome/browser/ui/views/frame/browser_frame.cc
diff --git a/chrome/browser/ui/views/frame/browser_frame.cc b/chrome/browser/ui/views/frame/browser_frame.cc
index e8f05e81fc21141faef468db88c386927979ce24..72334d32da7f6b3e9723e9a0dc9ea7c267284dc3 100644
--- a/chrome/browser/ui/views/frame/browser_frame.cc
+++ b/chrome/browser/ui/views/frame/browser_frame.cc
@@ -24,6 +24,7 @@
 #include "chrome/browser/ui/views/frame/top_container_view.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/pref_names.h"
 #include "ui/aura/root_window.h"
 #include "ui/aura/window.h"
 #include "ui/base/hit_test.h"
@@ -82,6 +83,12 @@ const gfx::FontList& BrowserFrame::GetTitleFontList() {
 }
 
 void BrowserFrame::InitBrowserFrame() {
+  use_custom_frame_pref_.Init(
+      prefs::kUseCustomChromeFrame,
+      browser_view_->browser()->profile()->GetPrefs(),
+      base::Bind(&BrowserFrame::OnUseCustomChromeFrameChanged,
+                 base::Unretained(this)));

[msw, 2014/02/11 19:32:59]

Using base::Unretained is generally unsafe. Is the callback discarded when this
BooleanPrefMember instance is destroyed (ie. when the BrowserFrame is being
destroyed)? Otherwise, this could crash if it were called after the BrowserFrame
instance were destroyed; which is unlikely, but possible. I think it'd be
preferable to use a WeakPtrFactory or some observer pattern, unless the
BooleanPrefMember destruction invalidates the callback.

[Matt Giuca, 2014/02/12 01:09:18]

Good question.

It looks like your last sentence is true: the destructor for PrefMember calls
RemovePrefObserver to remove itself automatically, which should ensure the
callback is not called after the destruction of `this`.

Again, I am copying this code from the existing GTK implementation which has
been working fine for years, and followed the same model (see BrowserWindowGtk).

+
   native_browser_frame_ =
       NativeBrowserFrameFactory::CreateNativeBrowserFrame(this, browser_view_);
   views::Widget::InitParams params;
@@ -131,8 +138,7 @@ void BrowserFrame::InitBrowserFrame() {
   params.wm_role_name = browser_view_->browser()->is_type_tabbed() ?
       std::string(kX11WindowRoleBrowser) : std::string(kX11WindowRolePopup);
 
-  params.remove_standard_frame =
-      !command_line.HasSwitch(switches::kUseSystemTitleBar);
+  params.remove_standard_frame = UseCustomFrame();
 #endif  // defined(OS_LINUX)
 
   Init(params);
@@ -172,6 +178,10 @@ views::View* BrowserFrame::GetFrameView() const {
   return browser_frame_view_;
 }
 
+bool BrowserFrame::UseCustomFrame() const {
+  return use_custom_frame_pref_.GetValue();
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserFrame, views::Widget overrides:
 
@@ -279,3 +289,10 @@ bool BrowserFrame::ShouldLeaveOffsetNearTopBorder() {
   return !IsMaximized();
 }
 #endif  // OS_WIN
+
+void BrowserFrame::OnUseCustomChromeFrameChanged() {
+  // Tell the window manager to add or remove system borders.
+  set_frame_type(UseCustomFrame() ? Widget::FRAME_TYPE_DEFAULT

[msw, 2014/02/11 19:32:59]

If the default is force native, won't this toggle do nothing? Maybe default is
not native, since DesktopWindowTreeHostX11::ShouldUseNativeFrame() and
NativeWidgetAura::ShouldUseNativeFrame() return false, but perhaps this toggle
should force between native and custom anyway.

[Matt Giuca, 2014/02/12 01:09:18]

The default is force custom, with a weird condition that if the native flag is
on when the window is created, it is default with a native frame.
Yeah, that's probably better. I have changed it so that on Linux, it is never
the default -- it is always set to FORCE_CUSTOM or FORCE_NATIVE depending on the
startup setting, and this toggle switches between the two. That avoids the weird
edge case involving DEFAULT mentioned above.

+                                  : Widget::FRAME_TYPE_FORCE_NATIVE);
+  FrameTypeChanged();
+}