arc-bridge: Enable ARC with a command-line flag

We have permission to remove the build time flag in favor of a runtime
flag for enabling ARC Bridge Service, conditional on committing to a
more stable and appropriate IPC mechanism for the bridge.

BUG=559654
TEST=components_unittests

Review URL: https://codereview.chromium.org/1466143002

Cr-Commit-Position: refs/heads/master@{#361484}

17 files changed, 44 insertions, 69 deletions

diff --git a/build/common.gypi b/build/common.gypi
index 6a06982..845ab5a 100644
--- a/build/common.gypi
+++ b/build/common.gypi
@@ -1563,9 +1563,6 @@
     'libjpeg_ijg_gyp_path': '<(DEPTH)/third_party/libjpeg/libjpeg.gyp',
     'libjpeg_turbo_gyp_path': '<(DEPTH)/third_party/libjpeg_turbo/libjpeg.gyp',
 
-    # ARC instance disabled by default.
-    'enable_arc%': 0,
-
     'conditions': [
       ['buildtype=="Official"', {
         # Continue to embed build meta data in Official builds, basically the
@@ -3106,9 +3103,6 @@
         # TODO: Enable on Windows too, http://crbug.com/404525
         'variables': { 'clang_warning_flags': ['-Wexit-time-destructors']},
       }],
-      ['<(enable_arc)==1', {
-        'defines': ['ENABLE_ARC=1'],
-      }],
       ['chromium_code==0', {
         'variables': {
           'clang_warning_flags': [
diff --git a/build/config/BUILD.gn b/build/config/BUILD.gn
index 643d4d9..9cc3756 100644
--- a/build/config/BUILD.gn
+++ b/build/config/BUILD.gn
@@ -279,9 +279,6 @@ config("feature_flags") {
   if (!fieldtrial_testing_like_official_build && !is_chrome_branded) {
     defines += [ "FIELDTRIAL_TESTING_ENABLED" ]
   }
-  if (enable_arc) {
-    defines += [ "ENABLE_ARC=1" ]
-  }
 }
 
 # Debug/release ----------------------------------------------------------------
diff --git a/build/config/features.gni b/build/config/features.gni
index f35f905..0249c44 100644
--- a/build/config/features.gni
+++ b/build/config/features.gni
@@ -202,6 +202,3 @@ enable_pre_sync_backup = is_win || is_mac || (is_linux && !is_chromeos)
 enable_webvr = is_android
 
 use_gio = is_desktop_linux && !is_headless
-
-# Chrome OS: whether to use ARC instances. Disabled by default.
-enable_arc = false
diff --git a/chrome/browser/chromeos/chrome_browser_main_chromeos.cc b/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
index 08bc59e..c69f48f 100644
--- a/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
+++ b/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
@@ -144,9 +144,7 @@
 #include "chrome/browser/chromeos/events/xinput_hierarchy_changed_event_listener.h"
 #endif
 
-#if defined(ENABLE_ARC)
 #include "components/arc/arc_bridge_service.h"
-#endif
 
 namespace chromeos {
 
@@ -390,14 +388,12 @@ void ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun() {
 
   wake_on_wifi_manager_.reset(new WakeOnWifiManager());
 
-#if defined(ENABLE_ARC)
   arc_bridge_service_.reset(new arc::ArcBridgeService(
       content::BrowserThread::GetMessageLoopProxyForThread(
           content::BrowserThread::IO),
       content::BrowserThread::GetMessageLoopProxyForThread(
           content::BrowserThread::FILE)));
   arc_bridge_service_->DetectAvailability();
-#endif
 
   ChromeBrowserMainPartsLinux::PreMainMessageLoopRun();
 }
@@ -712,9 +708,7 @@ void ChromeBrowserMainPartsChromeos::PostBrowserStart() {
 void ChromeBrowserMainPartsChromeos::PostMainMessageLoopRun() {
   BootTimesRecorder::Get()->AddLogoutTimeMarker("UIMessageLoopEnded", true);
 
-#if defined(ENABLE_ARC)
   arc_bridge_service_->Shutdown();
-#endif
 
   // Destroy the application name notifier for Kiosk mode.
   KioskModeIdleAppNameNotification::Shutdown();
diff --git a/chrome/browser/chromeos/chrome_browser_main_chromeos.h b/chrome/browser/chromeos/chrome_browser_main_chromeos.h
index 10b3a3b..0b54641 100644
--- a/chrome/browser/chromeos/chrome_browser_main_chromeos.h
+++ b/chrome/browser/chromeos/chrome_browser_main_chromeos.h
@@ -15,11 +15,9 @@ namespace session_manager {
 class SessionManager;
 }
 
-#if defined(ENABLE_ARC)
 namespace arc {
 class ArcBridgeService;
 }
-#endif
 
 namespace chromeos {
 
@@ -83,9 +81,7 @@ class ChromeBrowserMainPartsChromeos : public ChromeBrowserMainPartsLinux {
 
   scoped_refptr<chromeos::ExternalMetrics> external_metrics_;
 
-#if defined(ENABLE_ARC)
   scoped_ptr<arc::ArcBridgeService> arc_bridge_service_;
-#endif
 
   DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainPartsChromeos);
 };
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
index b9e8c12..be6d20f 100644
--- a/chrome/browser/ui/BUILD.gn
+++ b/chrome/browser/ui/BUILD.gn
@@ -196,13 +196,13 @@ source_set("ui") {
     sources += rebase_path(gypi_values.chrome_browser_ui_chromeos_sources,
                            ".",
                            "//chrome")
-    deps += [ "//chrome/browser/chromeos" ]
+    deps += [
+      "//chrome/browser/chromeos",
+      "//components/arc",
+    ]
   } else {
     defines += [ "FRAME_AVATAR_BUTTON=1" ]
   }
-  if (is_chromeos && enable_arc) {
-    deps += [ "//components/arc" ]
-  }
   if (use_cups) {
     configs += [ "//printing:cups" ]
   }
diff --git a/chrome/browser/ui/ash/chrome_shell_delegate.h b/chrome/browser/ui/ash/chrome_shell_delegate.h
index 13e363f..e109d82 100644
--- a/chrome/browser/ui/ash/chrome_shell_delegate.h
+++ b/chrome/browser/ui/ash/chrome_shell_delegate.h
@@ -17,7 +17,7 @@
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 
-#if defined(OS_CHROMEOS) && defined(ENABLE_ARC)
+#if defined(OS_CHROMEOS)
 #include "ash/shell_observer.h"
 #endif
 
@@ -91,7 +91,7 @@ class ChromeShellDelegate : public ash::ShellDelegate,
                const content::NotificationDetails& details) override;
 
  private:
-#if defined(OS_CHROMEOS) && defined(ENABLE_ARC)
+#if defined(OS_CHROMEOS)
   // An Observer to track session state and start/stop ARC accordingly.
   class ArcSessionObserver : public ash::ShellObserver {
    public:
@@ -123,9 +123,7 @@ class ChromeShellDelegate : public ash::ShellDelegate,
 #if defined(OS_CHROMEOS)
   scoped_ptr<chromeos::DisplayConfigurationObserver>
       display_configuration_observer_;
-#endif
 
-#if defined(OS_CHROMEOS) && defined(ENABLE_ARC)
   // An Observer to track session state and start/stop ARC accordingly.
   scoped_ptr<ArcSessionObserver> arc_session_observer_;
 #endif
diff --git a/chrome/browser/ui/ash/chrome_shell_delegate_chromeos.cc b/chrome/browser/ui/ash/chrome_shell_delegate_chromeos.cc
index c330a95..ef02593 100644
--- a/chrome/browser/ui/ash/chrome_shell_delegate_chromeos.cc
+++ b/chrome/browser/ui/ash/chrome_shell_delegate_chromeos.cc
@@ -36,16 +36,13 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/grit/generated_resources.h"
 #include "chromeos/chromeos_switches.h"
+#include "components/arc/arc_bridge_service.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/user_metrics.h"
 #include "ui/aura/window.h"
 #include "ui/base/ime/chromeos/input_method_manager.h"
 #include "ui/base/l10n/l10n_util.h"
 
-#if defined(ENABLE_ARC)
-#include "components/arc/arc_bridge_service.h"
-#endif
-
 namespace {
 
 void InitAfterFirstSessionStart() {
@@ -232,9 +229,7 @@ void ChromeShellDelegate::PreInit() {
   display_configuration_observer_.reset(
       new chromeos::DisplayConfigurationObserver());
 
-#if defined(ENABLE_ARC)
   arc_session_observer_.reset(new ArcSessionObserver);
-#endif
 
   chrome_user_metrics_recorder_.reset(new ChromeUserMetricsRecorder);
 }
@@ -242,11 +237,9 @@ void ChromeShellDelegate::PreInit() {
 void ChromeShellDelegate::PreShutdown() {
   display_configuration_observer_.reset();
 
-#if defined(ENABLE_ARC)
   // Remove the ARC observer now since it uses the ash::Shell instance in its
   // destructor, which is unavailable after PreShutdown() returns.
   arc_session_observer_.reset();
-#endif
 
   chrome_user_metrics_recorder_.reset();
 }
@@ -316,7 +309,6 @@ void ChromeShellDelegate::PlatformInit() {
                  content::NotificationService::AllSources());
 }
 
-#if defined(ENABLE_ARC)
 ChromeShellDelegate::ArcSessionObserver::ArcSessionObserver() {
   ash::Shell::GetInstance()->AddShellObserver(this);
 }
@@ -341,8 +333,10 @@ void ChromeShellDelegate::ArcSessionObserver::OnLoginStateChanged(
     case ash::user::LOGGED_IN_GUEST:
     case ash::user::LOGGED_IN_PUBLIC:
     case ash::user::LOGGED_IN_SUPERVISED:
-      arc::ArcBridgeService::Get()->HandleStartup();
+      if (arc::ArcBridgeService::GetEnabled(
+              base::CommandLine::ForCurrentProcess())) {
+        arc::ArcBridgeService::Get()->HandleStartup();
+      }
       break;
   }
 }
-#endif
diff --git a/chrome/chrome_browser_chromeos.gypi b/chrome/chrome_browser_chromeos.gypi
index 37250f2..0bd6ad5 100644
--- a/chrome/chrome_browser_chromeos.gypi
+++ b/chrome/chrome_browser_chromeos.gypi
@@ -1113,6 +1113,7 @@
         # browser_chromeos #includes power_supply_properties.pb.h directly.
         '../chromeos/chromeos.gyp:power_manager_proto',
         '../chromeos/ime/input_method.gyp:gencode',
+        '../components/components.gyp:arc',
         '../components/components.gyp:certificate_reporting',
         '../components/components.gyp:cloud_policy_proto',
         '../components/components.gyp:drive',
@@ -1265,11 +1266,6 @@
             '../ui/ozone/ozone.gyp:ozone',
           ],
         }],
-        ['enable_arc == 1', {
-          'dependencies': [
-            '../components/components.gyp:arc',
-          ],
-        }],
       ],
     },
     {
diff --git a/chromeos/chromeos_switches.cc b/chromeos/chromeos_switches.cc
index 2c1ce3d..2586f73 100644
--- a/chromeos/chromeos_switches.cc
+++ b/chromeos/chromeos_switches.cc
@@ -104,6 +104,9 @@ const char kDisableNetworkPortalNotification[] =
 const char kEafeUrl[] = "eafe-url";
 const char kEafePath[] = "eafe-path";
 
+// Enables starting the ARC instance upon session start.
+const char kEnableArc[] = "enable-arc";
+
 // Enables consumer management, which allows user to enroll, remotely lock and
 // locate the device.
 const char kEnableConsumerManagement[] = "enable-consumer-management";
diff --git a/chromeos/chromeos_switches.h b/chromeos/chromeos_switches.h
index 133a1cc..428bdb7 100644
--- a/chromeos/chromeos_switches.h
+++ b/chromeos/chromeos_switches.h
@@ -55,6 +55,7 @@ CHROMEOS_EXPORT extern const char kDisableVolumeAdjustSound[];
 CHROMEOS_EXPORT extern const char kDisableWakeOnWifi[];
 CHROMEOS_EXPORT extern const char kEafeUrl[];
 CHROMEOS_EXPORT extern const char kEafePath[];
+CHROMEOS_EXPORT extern const char kEnableArc[];
 CHROMEOS_EXPORT extern const char kEnableConsumerManagement[];
 CHROMEOS_EXPORT extern const char kEnableExtensionAssetsSharing[];
 CHROMEOS_EXPORT extern const char kEnableFirstRunUITransitions[];
diff --git a/components/BUILD.gn b/components/BUILD.gn
index caea660..038f18d 100644
--- a/components/BUILD.gn
+++ b/components/BUILD.gn
@@ -92,6 +92,7 @@ group("all_components") {
     deps += [
       "//components/about_handler",
       "//components/app_modal",
+      "//components/arc",
       "//components/auto_login_parser",
       "//components/autofill/content/browser",
       "//components/autofill/content/common",
@@ -225,15 +226,12 @@ group("all_components") {
 
     if (!is_chromeos) {
       deps -= [
+        "//components/arc",
         "//components/pairing",
         "//components/wifi_sync",
       ]
     }
 
-    if (is_chromeos && enable_arc) {
-      deps += [ "//components/arc" ]
-    }
-
     if (!use_ash) {
       deps -= [ "//components/exo" ]
     }
@@ -512,10 +510,10 @@ test("components_unittests") {
       }
     }
     if (is_chromeos) {
-      deps += [ "//components/ownership:unit_tests" ]
-    }
-    if (is_chromeos && enable_arc) {
-      deps += [ "//components/arc:unit_tests" ]
+      deps += [
+        "//components/arc:unit_tests",
+        "//components/ownership:unit_tests",
+      ]
     }
 
     # TODO(GYP) need this target.
diff --git a/components/arc/arc_bridge_service.cc b/components/arc/arc_bridge_service.cc
index c4890af..e2146c22 100644
--- a/components/arc/arc_bridge_service.cc
+++ b/components/arc/arc_bridge_service.cc
@@ -4,12 +4,14 @@
 
 #include "components/arc/arc_bridge_service.h"
 
+#include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/task_runner_util.h"
 #include "base/thread_task_runner_handle.h"
+#include "chromeos/chromeos_switches.h"
 #include "chromeos/dbus/dbus_method_call_status.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/session_manager_client.h"
@@ -57,6 +59,11 @@ ArcBridgeService* ArcBridgeService::Get() {
   return g_arc_bridge_service;
 }
 
+// static
+bool ArcBridgeService::GetEnabled(const base::CommandLine* command_line) {
+  return command_line->HasSwitch(chromeos::switches::kEnableArc);
+}
+
 void ArcBridgeService::DetectAvailability() {
   chromeos::SessionManagerClient* session_manager_client =
       chromeos::DBusThreadManager::Get()->GetSessionManagerClient();
@@ -233,7 +240,9 @@ void ArcBridgeService::OnInstanceStarted(bool success) {
 
 void ArcBridgeService::OnInstanceBootPhase(InstanceBootPhase phase) {
   DCHECK(origin_task_runner_->RunsTasksOnCurrentThread());
-  if (state_ != State::STARTING) {
+  // The state can be STARTING the first time this is called, and will then
+  // transition to READY after BRIDGE_READY has been passed.
+  if (state_ != State::STARTING && state_ != State::READY) {
     VLOG(1) << "StopInstance() called while connecting";
     return;
   }
diff --git a/components/arc/arc_bridge_service.h b/components/arc/arc_bridge_service.h
index 8d23d44..14fdf84 100644
--- a/components/arc/arc_bridge_service.h
+++ b/components/arc/arc_bridge_service.h
@@ -13,6 +13,10 @@
 #include "ipc/ipc_listener.h"
 #include "ipc/ipc_message.h"
 
+namespace base {
+class CommandLine;
+}
+
 namespace arc {
 
 // The Chrome-side service that handles ARC instances and ARC bridge creation.
@@ -84,6 +88,10 @@ class ArcBridgeService : public IPC::Listener {
   // called on the thread that this class was created on.
   static ArcBridgeService* Get();
 
+  // Return true if ARC has been enabled through a commandline
+  // switch.
+  static bool GetEnabled(const base::CommandLine* command_line);
+
   // DetectAvailability() should be called once D-Bus is available. It will
   // call CheckArcAvailability() on the session_manager. This can only be
   // called on the thread that this class was created on.
diff --git a/components/arc/arc_bridge_service_unittest.cc b/components/arc/arc_bridge_service_unittest.cc
index 3e14fbb..5b765ca 100644
--- a/components/arc/arc_bridge_service_unittest.cc
+++ b/components/arc/arc_bridge_service_unittest.cc
@@ -68,7 +68,7 @@ class ArcBridgeTest : public testing::Test, public ArcBridgeService::Observer {
     }
   }
 
-  void OnInstanceBootPhase(InstanceBootPhase boot_phase) {
+  void OnInstanceBootPhase(InstanceBootPhase boot_phase) override {
     boot_phase_ = boot_phase;
   }
 
diff --git a/components/components.gyp b/components/components.gyp
index 2729f9a..0d3d6de 100644
--- a/components/components.gyp
+++ b/components/components.gyp
@@ -149,16 +149,12 @@
     }],
     ['chromeos == 1', {
       'includes': [
+        'arc.gypi',
         'pairing.gypi',
         'timers.gypi',
         'wifi_sync.gypi',
       ],
     }],
-    ['chromeos == 1 and enable_arc == 1', {
-      'includes': [
-        'arc.gypi',
-      ],
-    }],
     ['OS == "win" or OS == "mac"', {
       'includes': [
         'wifi.gypi',
diff --git a/components/components_tests.gyp b/components/components_tests.gyp
index 5fcd7b3..be10498 100644
--- a/components/components_tests.gyp
+++ b/components/components_tests.gyp
@@ -1367,6 +1367,7 @@
         }],
         ['chromeos==1', {
           'sources': [
+            'arc/arc_bridge_service_unittest.cc',
             'pairing/message_buffer_unittest.cc',
             'timers/alarm_timer_unittest.cc',
             'wifi_sync/wifi_config_delegate_chromeos_unittest.cc',
@@ -1382,20 +1383,13 @@
           ],
           'dependencies': [
             '../chromeos/chromeos.gyp:chromeos_test_support',
+            'components.gyp:arc',
             'components.gyp:ownership',
             'components.gyp:pairing',
             'components.gyp:user_manager_test_support',
             'components.gyp:wifi_sync',
           ],
         }],
-        ['chromeos==1 and enable_arc==1', {
-          'sources': [
-            'arc/arc_bridge_service_unittest.cc',
-          ],
-          'dependencies': [
-            'components.gyp:arc',
-          ],
-        }],
         ['OS=="linux"', {
           'sources': [
             'metrics/serialization/serialization_utils_unittest.cc',