Condition the use of ChromeBrowserMainExtraPartsViewsLinux to !use_ozone

chrome/browser/ui/views/chrome_browser_main_extra_parts_views_linux.cc

 // Copyright 2016 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/chrome_browser_main_extra_parts_views_linux.h"
 
 #include "base/command_line.h"
 #include "base/run_loop.h"
 #include "chrome/browser/chrome_browser_main.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/ui/browser_list.h"
+#if !defined(USE_OZONE)
 #include "chrome/browser/ui/libgtkui/gtk_ui.h"
+#endif
 #include "chrome/browser/ui/simple_message_box.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/prefs/pref_service.h"
 #include "ui/aura/env.h"
 #include "ui/aura/window.h"
 #include "ui/base/ime/input_method_initializer.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/ui_base_switches.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/native_theme/native_theme_aura.h"
 #include "ui/native_theme/native_theme_dark_aura.h"
 #include "ui/views/linux_ui/linux_ui.h"
 #include "ui/views/widget/desktop_aura/desktop_screen.h"
+#if defined(USE_X11)

[Tom (Use chromium acct), 2016/11/01 18:10:07]

nit: Please move the gtk_ui.h include and the x11_desktop_handler.h include
below all of the normal includes

[tonikitoo, 2016/11/01 18:29:54]

Done.

 #include "ui/views/widget/desktop_aura/x11_desktop_handler.h"
+#endif
 #include "ui/views/widget/native_widget_aura.h"
 
 namespace {
 
 ui::NativeTheme* GetNativeThemeForWindow(aura::Window* window) {
   if (!window)
     return nullptr;
 
   Profile* profile = nullptr;
   // Window types not listed here (such as tooltips) will never use Chrome
@@ skipping 21 matching lines @@
   return ui::NativeThemeAura::instance();
 }
 
 }  // namespace
 
 ChromeBrowserMainExtraPartsViewsLinux::ChromeBrowserMainExtraPartsViewsLinux() {
 }
 
 ChromeBrowserMainExtraPartsViewsLinux::
     ~ChromeBrowserMainExtraPartsViewsLinux() {
   // X11DesktopHandler is destructed at this point, so we don't need to remove

[Tom (Use chromium acct), 2016/11/01 18:10:07]

nit: Surround this comment and DCHECK with #if defined(USE_X11)

[tonikitoo, 2016/11/01 18:29:54]

Done.

   // ourselves as an X11DesktopHandlerObserver
   DCHECK(!aura::Env::GetInstanceDontCreate());
 }
 
 void ChromeBrowserMainExtraPartsViewsLinux::PreEarlyInitialization() {
+#if !defined(USE_OZONE)
   // TODO(erg): Refactor this into a dlopen call when we add a GTK3 port.
   views::LinuxUI* gtk2_ui = BuildGtk2UI();
   gtk2_ui->SetNativeThemeOverride(base::Bind(&GetNativeThemeForWindow));
   views::LinuxUI::SetInstance(gtk2_ui);
+#endif
 }
 
 void ChromeBrowserMainExtraPartsViewsLinux::ToolkitInitialized() {
   ChromeBrowserMainExtraPartsViews::ToolkitInitialized();
+#if !defined(USE_OZONE)
   views::LinuxUI::instance()->Initialize();
+#endif
 }
 
 void ChromeBrowserMainExtraPartsViewsLinux::PreCreateThreads() {
+  // Intentionally do not call the parent class' PreCreateThreads

[Tom (Use chromium acct), 2016/11/01 18:10:06]

Add the check for !defined(USE_OZONE) in
ChromeBrowserMainExtraPartsViews::PreCreateThreads instead, and unconditionally
call ChromeBrowserMainExtraPartsViews::PreCreateThreads from here.

[tonikitoo, 2016/11/01 18:29:54]

Done.

+  // here, given that it instantiates DesktopScreen, which is not
+  // used in Ozone.
+#if !defined(USE_OZONE)
   ChromeBrowserMainExtraPartsViews::PreCreateThreads();
   views::LinuxUI::instance()->UpdateDeviceScaleFactor(
       display::Screen::GetScreen()->GetPrimaryDisplay().device_scale_factor());
+#endif
 
+#if defined(USE_X11)
   views::X11DesktopHandler::get()->AddObserver(this);
+#endif
 }
 
 void ChromeBrowserMainExtraPartsViewsLinux::PreProfileInit() {
   ChromeBrowserMainExtraPartsViews::PreProfileInit();
   // On the Linux desktop, we want to prevent the user from logging in as root,
   // so that we don't destroy the profile. Now that we have some minimal ui
   // initialized, check to see if we're running as root and bail if we are.
   if (getuid() != 0)
     return;
 
   const base::CommandLine& command_line =
       *base::CommandLine::ForCurrentProcess();
   if (command_line.HasSwitch(switches::kUserDataDir))
     return;
 
   base::string16 title = l10n_util::GetStringFUTF16(
       IDS_REFUSE_TO_RUN_AS_ROOT, l10n_util::GetStringUTF16(IDS_PRODUCT_NAME));
   base::string16 message = l10n_util::GetStringFUTF16(
       IDS_REFUSE_TO_RUN_AS_ROOT_2, l10n_util::GetStringUTF16(IDS_PRODUCT_NAME));
 
   chrome::ShowWarningMessageBox(NULL, title, message);
 
   // Avoids gpu_process_transport_factory.cc(153)] Check failed:
   // per_compositor_data_.empty() when quit is chosen.
   base::RunLoop().RunUntilIdle();
 
   exit(EXIT_FAILURE);
 }
 
 void ChromeBrowserMainExtraPartsViewsLinux::OnWorkspaceChanged(

[Tom (Use chromium acct), 2016/11/01 18:10:06]

I'm surprised this doesn't need to be surrounded with #if defined(USE_X11) as
well, since OnWorkspaceChanged is an override from X11DesktopHandlerObserver.

I guess the "right" thing to do would be to move x11_desktop_handler_observer.*
into the use_x11 portion of BUILD.gn and add the USE_X11 checks here

[tonikitoo, 2016/11/01 18:29:54]

I believe the reason it "works" is because X11DesktopHandlerObserver itself has
nothing X11-specific, although its named is prefixed with X11.

Its method ::OnWorkspaceChanged is only called from X11DesktopHandler thought,
so it ends up being an dead code in case of !USE_X11 (or USE_OZONE).

If you think cleaning it up this dependency is worth it, I could give it a shot.

[Tom (Use chromium acct), 2016/11/01 18:38:27]

I'll let thestig@ decide this one

[Lei Zhang, 2016/11/01 21:19:24]

Will OnWorkspaceChanged() ever be useful in an Ozone build? If you are not sure,
I think it's ok to leave it for now and have a few unused bits in the Ozone
build. You can clean it up later.

     const std::string& new_workspace) {
   BrowserList::MoveBrowsersInWorkspaceToFront(new_workspace);
 }