Prioritize ARC instance only when it is needed

Previously, the instance was prioritized when the OS fully started.
However, it turned out that the prioritization caused Chrome UI jank
on some boards.

This CL changes the way of prioritization more dynamic to prevent
such a jank.

BUG=chrome-os-partner:60946
TEST=try

Review-Url: https://codereview.chromium.org/2624513004
Cr-Commit-Position: refs/heads/master@{#442768}

chrome/browser/chromeos/BUILD.gn

@@ -229,6 +229,10 @@ source_set("chromeos") {
     "arc/auth/arc_manual_auth_code_fetcher.h",
     "arc/auth/arc_robot_auth_code_fetcher.cc",
     "arc/auth/arc_robot_auth_code_fetcher.h",
+    "arc/boot_phase_monitor/arc_boot_phase_monitor_bridge.cc",
+    "arc/boot_phase_monitor/arc_boot_phase_monitor_bridge.h",
+    "arc/boot_phase_monitor/arc_instance_throttle.cc",
+    "arc/boot_phase_monitor/arc_instance_throttle.h",
     "arc/downloads_watcher/arc_downloads_watcher_service.cc",
     "arc/downloads_watcher/arc_downloads_watcher_service.h",
     "arc/enterprise/arc_enterprise_reporting_service.cc",

chrome/browser/chromeos/arc/arc_service_launcher.cc

@@ -12,6 +12,7 @@
 #include "chrome/browser/chromeos/app_mode/arc/arc_kiosk_app_service.h"
 #include "chrome/browser/chromeos/arc/arc_auth_service.h"
 #include "chrome/browser/chromeos/arc/arc_session_manager.h"
+#include "chrome/browser/chromeos/arc/boot_phase_monitor/arc_boot_phase_monitor_bridge.h"
 #include "chrome/browser/chromeos/arc/downloads_watcher/arc_downloads_watcher_service.h"
 #include "chrome/browser/chromeos/arc/enterprise/arc_enterprise_reporting_service.h"
 #include "chrome/browser/chromeos/arc/fileapi/arc_file_system_service.h"
@@ -30,7 +31,6 @@
 #include "components/arc/arc_session_runner.h"
 #include "components/arc/audio/arc_audio_bridge.h"
 #include "components/arc/bluetooth/arc_bluetooth_bridge.h"
-#include "components/arc/boot_phase_monitor/arc_boot_phase_monitor_bridge.h"
 #include "components/arc/clipboard/arc_clipboard_bridge.h"
 #include "components/arc/crash_collector/arc_crash_collector_bridge.h"
 #include "components/arc/ime/arc_ime_service.h"

components/arc/boot_phase_monitor/arc_boot_phase_monitor_bridge.cc → chrome/browser/chromeos/arc/boot_phase_monitor/arc_boot_phase_monitor_bridge.cc

@@ -2,25 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/arc/boot_phase_monitor/arc_boot_phase_monitor_bridge.h"
+#include "chrome/browser/chromeos/arc/boot_phase_monitor/arc_boot_phase_monitor_bridge.h"
 
 #include "base/logging.h"
+#include "chrome/browser/chromeos/arc/boot_phase_monitor/arc_instance_throttle.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/session_manager_client.h"
 #include "components/arc/arc_bridge_service.h"
 
 namespace arc {
 
-namespace {
-
-void PrioritizeArcInstanceCallback(bool success) {
-  VLOG(2) << "Finished prioritizing the instance: result=" << success;
-  if (!success)
-    LOG(WARNING) << "Failed to prioritize ARC";
-}
-
-}  // namespace
-
 ArcBootPhaseMonitorBridge::ArcBootPhaseMonitorBridge(
     ArcBridgeService* bridge_service)
     : ArcService(bridge_service), binding_(this) {
@@ -48,11 +39,14 @@ void ArcBootPhaseMonitorBridge::OnInstanceClosed() {
 void ArcBootPhaseMonitorBridge::OnBootCompleted() {
   DCHECK(thread_checker_.CalledOnValidThread());
   VLOG(2) << "OnBootCompleted";
+
   chromeos::SessionManagerClient* session_manager_client =
       chromeos::DBusThreadManager::Get()->GetSessionManagerClient();
-  session_manager_client->PrioritizeArcInstance(
-      base::Bind(PrioritizeArcInstanceCallback));
   session_manager_client->EmitArcBooted();
+
+  // Start monitoring window activation changes to prioritize/throttle the
+  // container when needed.
+  throttle_ = base::MakeUnique<ArcInstanceThrottle>();
 }
 
 }  // namespace arc

components/arc/boot_phase_monitor/arc_boot_phase_monitor_bridge.h → chrome/browser/chromeos/arc/boot_phase_monitor/arc_boot_phase_monitor_bridge.h

@@ -2,8 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef COMPONENTS_ARC_BOOT_PHASE_MONITOR_ARC_BOOT_PHASE_MONITOR_BRIDGE_H_
-#define COMPONENTS_ARC_BOOT_PHASE_MONITOR_ARC_BOOT_PHASE_MONITOR_BRIDGE_H_
+#ifndef CHROME_BROWSER_CHROMEOS_ARC_BOOT_PHASE_MONITOR_ARC_BOOT_PHASE_MONITOR_BRIDGE_H_
+#define CHROME_BROWSER_CHROMEOS_ARC_BOOT_PHASE_MONITOR_ARC_BOOT_PHASE_MONITOR_BRIDGE_H_
+
+#include <memory>
 
 #include "base/macros.h"
 #include "base/threading/thread_checker.h"
@@ -15,6 +17,7 @@
 namespace arc {
 
 class ArcBridgeService;
+class ArcInstanceThrottle;
 
 // Receives boot phase notifications from ARC.
 class ArcBootPhaseMonitorBridge
@@ -34,6 +37,7 @@ class ArcBootPhaseMonitorBridge
 
  private:
   mojo::Binding<mojom::BootPhaseMonitorHost> binding_;
+  std::unique_ptr<ArcInstanceThrottle> throttle_;
 
   base::ThreadChecker thread_checker_;
 
@@ -42,4 +46,4 @@ class ArcBootPhaseMonitorBridge
 
 }  // namespace arc
 
-#endif  // COMPONENTS_ARC_BOOT_PHASE_MONITOR_ARC_BOOT_PHASE_MONITOR_BRIDGE_H_
+#endif  // CHROME_BROWSER_CHROMEOS_ARC_BOOT_PHASE_MONITOR_ARC_BOOT_PHASE_MONITOR_BRIDGE_H_

chrome/browser/chromeos/arc/boot_phase_monitor/arc_instance_throttle.cc

+// Copyright 2017 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/chromeos/arc/boot_phase_monitor/arc_instance_throttle.h"
+
+#include "ash/common/wm_shell.h"
+#include "ash/common/wm_window.h"
+#include "ash/shared/app_types.h"
+#include "base/bind.h"
+#include "base/logging.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/session_manager_client.h"
+
+namespace arc {
+
+namespace {
+
+void OnDBusReply(bool is_prioritize, bool success) {
+  if (success)
+    return;
+  const char* message = is_prioritize ? "unprioritize" : "prioritize";
+  LOG(WARNING) << "Failed to " << message << " the instance";
+}
+
+bool IsArcAppWindow(ash::WmWindow* active) {
+  DCHECK(active);
+  return active->GetAppType() == static_cast<int>(ash::AppType::ARC_APP);
+}
+
+void ThrottleInstanceIfNeeded(ash::WmWindow* active) {
+  chromeos::SessionManagerClient* session_manager_client =
+      chromeos::DBusThreadManager::Get()->GetSessionManagerClient();
+  if (!session_manager_client) {
+    LOG(WARNING) << "SessionManagerClient is not available";
+    return;
+  }
+  if (!active || !IsArcAppWindow(active)) {
+    // TODO(yusukes): Call session_manager_client->UnprioritizeArcInstance()
+    // once it's ready.
+  } else {
+    session_manager_client->PrioritizeArcInstance(
+        base::Bind(OnDBusReply, true));
+  }
+}
+
+}  // namespace
+
+ArcInstanceThrottle::ArcInstanceThrottle() {
+  ash::WmShell* shell = ash::WmShell::Get();
+  DCHECK(shell);
+  shell->AddActivationObserver(this);
+  ThrottleInstanceIfNeeded(shell->GetActiveWindow());
+}
+
+ArcInstanceThrottle::~ArcInstanceThrottle() {
+  ash::WmShell* shell = ash::WmShell::Get();
+  if (shell)
+    shell->RemoveActivationObserver(this);
+}
+
+void ArcInstanceThrottle::OnWindowActivated(ash::WmWindow* gained_active,
+                                            ash::WmWindow* lost_active) {
+  ThrottleInstanceIfNeeded(gained_active);
+}
+
+}  // namespace arc

chrome/browser/chromeos/arc/boot_phase_monitor/arc_instance_throttle.h

+// Copyright 2017 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.
+
+#ifndef CHROME_BROWSER_CHROMEOS_ARC_BOOT_PHASE_MONITOR_ARC_INSTANCE_THROTTLE_H_
+#define CHROME_BROWSER_CHROMEOS_ARC_BOOT_PHASE_MONITOR_ARC_INSTANCE_THROTTLE_H_
+
+#include "ash/common/wm_activation_observer.h"
+#include "base/macros.h"
+
+namespace arc {
+
+// A class that watches window activations and prioritizes the ARC instance when
+// one of ARC windows is activated. The class also unprioritizes the instance
+// when non-ARC window such as Chrome is activated.
+class ArcInstanceThrottle : public ash::WmActivationObserver {
+ public:
+  ArcInstanceThrottle();
+  ~ArcInstanceThrottle() override;
+
+  // ash::WmActivationObserver overrides:
+  void OnWindowActivated(ash::WmWindow* gained_active,
+                         ash::WmWindow* lost_active) override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ArcInstanceThrottle);
+};
+
+}  // namespace arc
+
+#endif  // CHROME_BROWSER_CHROMEOS_ARC_BOOT_PHASE_MONITOR_ARC_INSTANCE_THROTTLE_H_

components/arc/BUILD.gn

@@ -17,8 +17,6 @@ static_library("arc") {
     "bluetooth/bluetooth_struct_traits.h",
     "bluetooth/bluetooth_type_converters.cc",
     "bluetooth/bluetooth_type_converters.h",
-    "boot_phase_monitor/arc_boot_phase_monitor_bridge.cc",
-    "boot_phase_monitor/arc_boot_phase_monitor_bridge.h",
     "clipboard/arc_clipboard_bridge.cc",
     "clipboard/arc_clipboard_bridge.h",
     "crash_collector/arc_crash_collector_bridge.cc",