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

chrome/browser/ui/views/frame/browser_frame.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/frame/browser_frame.h"
 
 #include "ash/shell.h"
 #include "base/command_line.h"
 #include "base/debug/leak_annotations.h"
 #include "base/i18n/rtl.h"
 #include "chrome/browser/app_mode/app_mode_utils.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window_state.h"
 #include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h"
 #include "chrome/browser/ui/views/frame/browser_root_view.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/frame/immersive_mode_controller.h"
 #include "chrome/browser/ui/views/frame/native_browser_frame.h"
 #include "chrome/browser/ui/views/frame/native_browser_frame_factory.h"
 #include "chrome/browser/ui/views/frame/system_menu_model_builder.h"
 #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"
 #include "ui/base/theme_provider.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/screen.h"
 #include "ui/views/controls/menu/menu_runner.h"
 #include "ui/views/widget/native_widget.h"
 
 #if defined(OS_WIN) && !defined(USE_AURA)
@@ skipping 38 matching lines @@
   static const gfx::FontList* title_font_list = new gfx::FontList();
 #else
   static const gfx::FontList* title_font_list =
       new gfx::FontList(views::NativeWidgetWin::GetWindowTitleFontList());
 #endif
   ANNOTATE_LEAKING_OBJECT_PTR(title_font_list);
   return *title_font_list;
 }
 
 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;
   params.delegate = browser_view_;
   params.native_widget = native_browser_frame_->AsNativeWidget();
   if (browser_view_->browser()->is_type_tabbed()) {
     // Typed panel/popup can only return a size once the widget has been
     // created.
     chrome::GetSavedWindowBoundsAndShowState(browser_view_->browser(),
                                              &params.bounds,
@@ skipping 29 matching lines @@
     // file.
     const std::string user_data_dir =
         command_line.GetSwitchValueNative(switches::kUserDataDir);
     params.wm_class_name += " (" + user_data_dir + ")";
   }
   const char kX11WindowRoleBrowser[] = "browser";
   const char kX11WindowRolePopup[] = "pop-up";
   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);
 
   if (!native_browser_frame_->UsesNativeSystemMenu()) {
     DCHECK(non_client_view());
     non_client_view()->set_context_menu_controller(this);
   }
 }
 
@@ skipping 19 matching lines @@
 }
 
 void BrowserFrame::UpdateThrobber(bool running) {
   browser_frame_view_->UpdateThrobber(running);
 }
 
 views::View* BrowserFrame::GetFrameView() const {
   return browser_frame_view_;
 }
 
+bool BrowserFrame::UseCustomFrame() const {
+  return use_custom_frame_pref_.GetValue();
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserFrame, views::Widget overrides:
 
 views::internal::RootView* BrowserFrame::CreateRootView() {
   root_view_ = new BrowserRootView(browser_view_, this);
   return root_view_;
 }
 
 views::NonClientFrameView* BrowserFrame::CreateNonClientFrameView() {
   browser_frame_view_ =
@@ skipping 87 matching lines @@
 
 NewAvatarButton* BrowserFrame::GetNewAvatarMenuButton() {
   return browser_frame_view_->new_avatar_button();
 }
 
 #if !defined(OS_WIN) || defined(USE_AURA)
 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();
+}