Remember the orientation request without a window.

chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc

Index: chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
diff --git a/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc b/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
index eb5a600576234ab8e1fd1e5c0e53093f4203743d..3c7b7f371706259133fa82ea849b31858c3b709d 100644
--- a/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
+++ b/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
@@ -88,12 +88,12 @@ blink::WebScreenOrientationLockType BlinkOrientationLockFromMojom(
 }
 
 int GetWindowTaskId(aura::Window* window) {
-  const std::string window_app_id = exo::ShellSurface::GetApplicationId(window);
-  if (window_app_id.empty())
+  const std::string arc_app_id = exo::ShellSurface::GetApplicationId(window);
+  if (arc_app_id.empty())
     return -1;
 
   int task_id = -1;
-  if (sscanf(window_app_id.c_str(), "org.chromium.arc.%d", &task_id) != 1)
+  if (sscanf(arc_app_id.c_str(), "org.chromium.arc.%d", &task_id) != 1)
     return -1;
 
   return task_id;
@@ -101,13 +101,52 @@ int GetWindowTaskId(aura::Window* window) {
 
 }  // namespace
 
+// The information about the arc application window which has to be kept
+// even when its AppWindow is not present.
+class ArcAppWindowLauncherController::AppWindowInfo {
+ public:
+  explicit AppWindowInfo(const std::string& shelf_app_id)
+      : shelf_app_id_(shelf_app_id) {}
+  ~AppWindowInfo() {}
+
+  const std::string& shelf_app_id() const { return shelf_app_id_; }
+
+  bool has_requested_orientation_lock() const {
+    return has_requested_orientation_lock_;
+  }
+
+  void set_requested_orientation_lock(arc::mojom::OrientationLock lock) {
+    has_requested_orientation_lock_ = true;
+    requested_orientation_lock_ = lock;
+  }
+
+  arc::mojom::OrientationLock requested_orientation_lock() const {
+    return requested_orientation_lock_;
+  }
+
+  void set_app_window(std::unique_ptr<AppWindow> window) {
+    app_window_ = std::move(window);
+  }
+
+  AppWindow* app_window() { return app_window_.get(); }
+
+ private:
+  std::string shelf_app_id_;
+  bool has_requested_orientation_lock_ = false;
+  arc::mojom::OrientationLock requested_orientation_lock_ =
+      arc::mojom::OrientationLock::NONE;
+  std::unique_ptr<AppWindow> app_window_;
+
+  DISALLOW_COPY_AND_ASSIGN(AppWindowInfo);
+};
+
+// A ui::BaseWindow for a chromeos launcher to control ARC applications.
 class ArcAppWindowLauncherController::AppWindow : public ui::BaseWindow {
  public:
   AppWindow(int task_id,
-            const std::string app_id,
             views::Widget* widget,
             ArcAppWindowLauncherController* owner)
-      : task_id_(task_id), app_id_(app_id), widget_(widget), owner_(owner) {}
+      : task_id_(task_id), widget_(widget), owner_(owner) {}
   ~AppWindow() {}
 
   void SetController(ArcAppWindowLauncherItemController* controller) {
@@ -134,8 +173,6 @@ class ArcAppWindowLauncherController::AppWindow : public ui::BaseWindow {
 
   ArcAppWindowLauncherItemController* controller() { return controller_; }
 
-  const std::string& app_id() { return app_id_; }
-
   // ui::BaseWindow:
   bool IsActive() const override {
     return widget_->IsActive() && owner_->active_task_id_ == task_id_;
@@ -204,23 +241,9 @@ class ArcAppWindowLauncherController::AppWindow : public ui::BaseWindow {
 
   void SetAlwaysOnTop(bool always_on_top) override { NOTREACHED(); }
 
-  arc::mojom::OrientationLock requested_orientation_lock() const {
-    return requested_orientation_lock_;
-  }
-
-  void set_requested_orientation_lock(arc::mojom::OrientationLock lock) {
-    has_requested_orientation_lock_ = true;
-    requested_orientation_lock_ = lock;
-  }
-
-  bool has_requested_orientation_lock() const {
-    return has_requested_orientation_lock_;
-  }
-
  private:
   int task_id_;
   ash::ShelfID shelf_id_ = 0;
-  std::string app_id_;
   FullScreenMode fullscreen_mode_ = FullScreenMode::NOT_DEFINED;
   // Unowned pointers
   views::Widget* const widget_;
@@ -228,10 +251,6 @@ class ArcAppWindowLauncherController::AppWindow : public ui::BaseWindow {
   ArcAppWindowLauncherItemController* controller_ = nullptr;
   // Unowned pointer, represents host Arc window.
 
-  arc::mojom::OrientationLock requested_orientation_lock_ =
-      arc::mojom::OrientationLock::NONE;
-  bool has_requested_orientation_lock_ = false;
-
   DISALLOW_COPY_AND_ASSIGN(AppWindow);
 };
 
@@ -278,19 +297,16 @@ void ArcAppWindowLauncherController::ActiveUserChanged(
 
     // Make sure that we created items for all apps, not only which have a
     // window.
-    for (const auto& it : task_id_to_shelf_app_id_)
-      AttachControllerToTask(it.second, it.first);
+    for (const auto& info : task_id_to_app_window_info_)
+      AttachControllerToTask(info.second->shelf_app_id(), info.first);
 
     // Update active status.
     OnTaskSetActive(active_task_id_);
   } else {
     // Remove all Arc apps and destroy its controllers. There is no mapping
     // task id to app window because it is not safe when controller is missing.
-    for (auto& it : task_id_to_app_window_) {
-      AppWindow* app_window = it.second.get();
-      UnregisterApp(app_window, true);
-    }
-    task_id_to_app_window_.clear();
+    for (auto& it : task_id_to_app_window_info_)
+      UnregisterApp(it.second.get(), true);
 
     // Some controllers might have no windows attached, for example background
     // task when foreground tasks is in full screen.
@@ -340,26 +356,30 @@ void ArcAppWindowLauncherController::OnWindowDestroying(aura::Window* window) {
   observed_windows_.erase(it);
   window->RemoveObserver(this);
 
-  auto it_app_window =
-      std::find_if(task_id_to_app_window_.begin(), task_id_to_app_window_.end(),
-                   [window](const TaskIdToAppWindow::value_type& pair) {
-                     return pair.second->GetNativeWindow() == window;
-                   });
-  if (it_app_window != task_id_to_app_window_.end()) {
+  auto info_it = std::find_if(
+      task_id_to_app_window_info_.begin(), task_id_to_app_window_info_.end(),
+      [window](TaskIdToAppWindowInfo::value_type& pair) {
+        return pair.second->app_window() &&
+               pair.second->app_window()->GetNativeWindow() == window;
+      });
+  if (info_it != task_id_to_app_window_info_.end()) {
     // Note, window may be recreated in some cases, so do not close controller
     // on window destroying. Controller will be closed onTaskDestroyed event
     // which is generated when actual task is destroyed.
-    UnregisterApp(it_app_window->second.get(), false);
-    task_id_to_app_window_.erase(it_app_window);
+    UnregisterApp(info_it->second.get(), false);
   }
 }
 
+ArcAppWindowLauncherController::AppWindowInfo*
+ArcAppWindowLauncherController::GetAppWindowInfoForTask(int task_id) {
+  auto it = task_id_to_app_window_info_.find(task_id);

[khmel, 2016/10/03 15:24:13]

nit: const auto...?

[oshima, 2016/10/04 00:28:31]

Done.

+  return it == task_id_to_app_window_info_.end() ? nullptr : it->second.get();
+}
+
 ArcAppWindowLauncherController::AppWindow*
 ArcAppWindowLauncherController::GetAppWindowForTask(int task_id) {
-  TaskIdToAppWindow::iterator it = task_id_to_app_window_.find(task_id);
-  if (it == task_id_to_app_window_.end())
-    return nullptr;
-  return it->second.get();
+  AppWindowInfo* info = GetAppWindowInfoForTask(task_id);
+  return info ? info->app_window() : nullptr;
 }
 
 void ArcAppWindowLauncherController::AttachControllerToWindowsIfNeeded() {
@@ -386,39 +406,38 @@ void ArcAppWindowLauncherController::AttachControllerToWindowIfNeeded(
     return;
 
   // Create controller if we have task info.
-  TaskIdToShelfAppIdMap::iterator it = task_id_to_shelf_app_id_.find(task_id);
-  if (it == task_id_to_shelf_app_id_.end())
+  AppWindowInfo* info = GetAppWindowInfoForTask(task_id);
+  if (!info) {
+    LOG(ERROR) << "Could not find AppWindowInfo for task:" << task_id;

[khmel, 2016/10/03 15:24:13]

This is not an error. There is a race when window created and new task message
delivered. There is a unit_test for this. Please remove error logging.
(::OnWindowVisibilityChanged may be called by new task id may not arrived yet,
that is normal case)

[oshima, 2016/10/04 00:28:31]

changed to VLOG(1)

     return;
-
-  const std::string& app_id = it->second;
+  }
 
   views::Widget* widget = views::Widget::GetWidgetForNativeWindow(window);
   DCHECK(widget);
-  std::unique_ptr<AppWindow> app_window(
-      new AppWindow(task_id, app_id, widget, this));
-  RegisterApp(app_window.get());
-  DCHECK(app_window->controller());
+  info->set_app_window(base::MakeUnique<AppWindow>(task_id, widget, this));

[khmel, 2016/10/03 15:24:13]

nit: Please add DCHECK_EQ(nullptr, info->get_app_window());

[oshima, 2016/10/04 00:28:31]

Done.

+  RegisterApp(info);
+  DCHECK(info->app_window()->controller());
   ash::WmWindowAura::Get(window)->SetIntProperty(
-      ash::WmWindowProperty::SHELF_ID, app_window->shelf_id());
+      ash::WmWindowProperty::SHELF_ID, info->app_window()->shelf_id());
   if (ash::WmShell::Get()
           ->maximize_mode_controller()
           ->IsMaximizeModeWindowManagerEnabled()) {
-    SetOrientationLockForAppWindow(app_window.get());
+    SetOrientationLockForAppWindow(info->app_window());
   }
-  task_id_to_app_window_[task_id] = std::move(app_window);
 }
 
 void ArcAppWindowLauncherController::OnAppReadyChanged(
-    const std::string& app_id,
+    const std::string& arc_app_id,
     bool ready) {
   if (!ready)
-    OnAppRemoved(app_id);
+    OnAppRemoved(arc_app_id);
 }
 
-void ArcAppWindowLauncherController::OnAppRemoved(const std::string& app_id) {
-  const std::string shelf_app_id = GetShelfAppIdFromArcAppId(app_id);
+void ArcAppWindowLauncherController::OnAppRemoved(
+    const std::string& arc_app_id) {
+  const std::string shelf_app_id = GetShelfAppIdFromArcAppId(arc_app_id);
 
-  AppControllerMap::const_iterator it = app_controller_map_.find(shelf_app_id);
+  const auto it = app_controller_map_.find(shelf_app_id);
   if (it == app_controller_map_.end())
     return;
 
@@ -441,10 +460,11 @@ void ArcAppWindowLauncherController::OnTaskCreated(
     const std::string& package_name,
     const std::string& activity_name) {
   DCHECK(!GetAppWindowForTask(task_id));
-  const std::string shelf_app_id = GetShelfAppIdFromArcAppId(
-      ArcAppListPrefs::GetAppId(package_name, activity_name));
-  task_id_to_shelf_app_id_[task_id] = shelf_app_id;
-
+  const std::string arc_app_id =
+      ArcAppListPrefs::GetAppId(package_name, activity_name);
+  const std::string shelf_app_id = GetShelfAppIdFromArcAppId(arc_app_id);
+  task_id_to_app_window_info_[task_id] =
+      base::MakeUnique<AppWindowInfo>(shelf_app_id);
   // Don't create shelf icon for non-primary user.
   if (observed_profile_ != owner()->GetProfile())
     return;
@@ -458,21 +478,16 @@ void ArcAppWindowLauncherController::OnTaskCreated(
 }
 
 void ArcAppWindowLauncherController::OnTaskDestroyed(int task_id) {
-  auto it = task_id_to_app_window_.find(task_id);
-  if (it != task_id_to_app_window_.end()) {
-    AppWindow* app_window = it->second.get();
-    UnregisterApp(app_window, true);
-    task_id_to_app_window_.erase(it);
-  }
+  auto it = task_id_to_app_window_info_.find(task_id);
+  if (it == task_id_to_app_window_info_.end())
+    return;
+  UnregisterApp(it->second.get(), true);
 
   // Check if we may close controller now, at this point we can safely remove
   // controllers without window.
-  auto it_app_id = task_id_to_shelf_app_id_.find(task_id);
-  if (it_app_id == task_id_to_shelf_app_id_.end())
-    return;
+  const std::string& shelf_app_id = it->second->shelf_app_id();
 
-  const std::string& app_id = it_app_id->second;
-  AppControllerMap::iterator it_controller = app_controller_map_.find(app_id);
+  const auto it_controller = app_controller_map_.find(shelf_app_id);
   if (it_controller != app_controller_map_.end()) {
     ArcAppWindowLauncherItemController* controller = it_controller->second;
     controller->RemoveTaskId(task_id);
@@ -482,7 +497,7 @@ void ArcAppWindowLauncherController::OnTaskDestroyed(int task_id) {
     }
   }
 
-  task_id_to_shelf_app_id_.erase(it_app_id);
+  task_id_to_app_window_info_.erase(it);
 }
 
 void ArcAppWindowLauncherController::OnTaskSetActive(int32_t task_id) {
@@ -491,27 +506,24 @@ void ArcAppWindowLauncherController::OnTaskSetActive(int32_t task_id) {
     return;
   }
 
-  TaskIdToAppWindow::iterator previous_active_app_it =
-      task_id_to_app_window_.find(active_task_id_);
-  if (previous_active_app_it != task_id_to_app_window_.end()) {
-    owner()->SetItemStatus(previous_active_app_it->second->shelf_id(),
+  AppWindow* previous_app_window = GetAppWindowForTask(active_task_id_);
+  if (previous_app_window) {
+    owner()->SetItemStatus(previous_app_window->shelf_id(),
                            ash::STATUS_RUNNING);
-    previous_active_app_it->second->SetFullscreenMode(
-        previous_active_app_it->second->widget() &&
-                previous_active_app_it->second->widget()->IsFullscreen()
+    previous_app_window->SetFullscreenMode(
+        previous_app_window->widget() &&
+                previous_app_window->widget()->IsFullscreen()
             ? FullScreenMode::ACTIVE
             : FullScreenMode::NON_ACTIVE);
   }
 
   active_task_id_ = task_id;
 
-  TaskIdToAppWindow::iterator new_active_app_it =
-      task_id_to_app_window_.find(active_task_id_);
-  if (new_active_app_it != task_id_to_app_window_.end()) {
+  AppWindow* current_app_window = GetAppWindowForTask(task_id);
+  if (current_app_window) {
     owner()->SetItemStatus(
-        new_active_app_it->second->shelf_id(),
-        new_active_app_it->second->widget() &&
-                new_active_app_it->second->widget()->IsActive()
+        current_app_window->shelf_id(),
+        current_app_window->widget() && current_app_window->IsActive()
             ? ash::STATUS_ACTIVE
             : ash::STATUS_RUNNING);
     // TODO(reveman): Figure out how to support fullscreen in interleaved
@@ -527,17 +539,19 @@ void ArcAppWindowLauncherController::OnTaskSetActive(int32_t task_id) {
 void ArcAppWindowLauncherController::OnTaskOrientationLockRequested(
     int32_t task_id,
     const arc::mojom::OrientationLock orientation_lock) {
-  // Don't save to AppInfo because this is requested in runtime.
-  TaskIdToAppWindow::iterator app_it = task_id_to_app_window_.find(task_id);
-  if (app_it == task_id_to_app_window_.end())
+  // Don't save to AppInfo in prefs because this is requested in runtime.
+  AppWindowInfo* info = GetAppWindowInfoForTask(task_id);
+  DCHECK(info);
+  if (!info)
     return;
-  AppWindow* app_window = app_it->second.get();
-  app_window->set_requested_orientation_lock(orientation_lock);
+  info->set_requested_orientation_lock(orientation_lock);
 
   if (ash::WmShell::Get()
           ->maximize_mode_controller()
           ->IsMaximizeModeWindowManagerEnabled()) {
-    SetOrientationLockForAppWindow(app_window);
+    AppWindow* app_window = info->app_window();
+    if (app_window)
+      SetOrientationLockForAppWindow(app_window);
   }
 }
 
@@ -549,9 +563,13 @@ ArcAppWindowLauncherController::ControllerForWindow(aura::Window* window) {
     return app_window->controller();
   }
 
-  for (auto& it : task_id_to_app_window_) {
-    if (it.second->widget() == views::Widget::GetWidgetForNativeWindow(window))
-      return it.second->controller();
+  for (auto& it : task_id_to_app_window_info_) {
+    AppWindow* app_window = it.second->app_window();
+    if (app_window &&
+        app_window->widget() ==
+            views::Widget::GetWidgetForNativeWindow(window)) {
+      return it.second->app_window()->controller();
+    }
   }
 
   return nullptr;
@@ -565,8 +583,11 @@ void ArcAppWindowLauncherController::OnWindowActivated(
 }
 
 void ArcAppWindowLauncherController::OnMaximizeModeStarted() {
-  for (auto& it : task_id_to_app_window_)
-    SetOrientationLockForAppWindow(it.second.get());
+  for (auto& it : task_id_to_app_window_info_) {
+    AppWindow* app_window = it.second->app_window();
+    if (app_window)
+      SetOrientationLockForAppWindow(app_window);
+  }
 }
 
 void ArcAppWindowLauncherController::OnMaximizeModeEnded() {
@@ -599,7 +620,7 @@ ArcAppWindowLauncherItemController*
 ArcAppWindowLauncherController::AttachControllerToTask(
     const std::string& shelf_app_id,
     int task_id) {
-  AppControllerMap::const_iterator it = app_controller_map_.find(shelf_app_id);
+  const auto it = app_controller_map_.find(shelf_app_id);
   if (it != app_controller_map_.end()) {
     DCHECK_EQ(it->second->app_id(), shelf_app_id);
     it->second->AddTaskId(task_id);
@@ -622,15 +643,17 @@ ArcAppWindowLauncherController::AttachControllerToTask(
   return controller;
 }
 
-void ArcAppWindowLauncherController::RegisterApp(AppWindow* app_window) {
-  const std::string app_id = app_window->app_id();
-  DCHECK(!app_id.empty());
-
+void ArcAppWindowLauncherController::RegisterApp(
+    AppWindowInfo* app_window_info) {
+  const std::string shelf_app_id = app_window_info->shelf_app_id();
+  DCHECK(!shelf_app_id.empty());
+  AppWindow* app_window = app_window_info->app_window();
   ArcAppWindowLauncherItemController* controller =
-      AttachControllerToTask(app_id, app_window->task_id());
+      AttachControllerToTask(shelf_app_id, app_window->task_id());
   DCHECK(controller);
 
-  const ash::ShelfID shelf_id = shelf_delegate_->GetShelfIDForAppID(app_id);
+  const ash::ShelfID shelf_id =
+      shelf_delegate_->GetShelfIDForAppID(shelf_app_id);
   DCHECK(shelf_id);
 
   controller->AddWindow(app_window);
@@ -639,12 +662,16 @@ void ArcAppWindowLauncherController::RegisterApp(AppWindow* app_window) {
   app_window->set_shelf_id(shelf_id);
 }
 
-void ArcAppWindowLauncherController::UnregisterApp(AppWindow* app_window,
-                                                   bool close_controller) {
-  const std::string app_id = app_window->app_id();
-
-  DCHECK(!app_id.empty());
-  AppControllerMap::iterator it = app_controller_map_.find(app_id);
+void ArcAppWindowLauncherController::UnregisterApp(
+    AppWindowInfo* app_window_info,
+    bool close_controller) {
+  AppWindow* app_window = app_window_info->app_window();
+  if (!app_window)
+    return;
+  const std::string shelf_app_id = app_window_info->shelf_app_id();

[khmel, 2016/10/03 15:24:13]

const std::string&

[oshima, 2016/10/04 00:28:32]

Done.

+  DCHECK(app_window);
+  DCHECK(!shelf_app_id.empty());
+  const auto it = app_controller_map_.find(shelf_app_id);
   CHECK(it != app_controller_map_.end());
 
   ArcAppWindowLauncherItemController* controller = it->second;
@@ -655,6 +682,7 @@ void ArcAppWindowLauncherController::UnregisterApp(AppWindow* app_window,
     app_controller_map_.erase(it);
   }
   app_window->ResetController();
+  app_window_info->set_app_window(nullptr);
 }
 
 void ArcAppWindowLauncherController::SetOrientationLockForAppWindow(
@@ -663,13 +691,15 @@ void ArcAppWindowLauncherController::SetOrientationLockForAppWindow(
       ash::WmLookup::Get()->GetWindowForWidget(app_window->widget());
   if (!window)
     return;
+  AppWindowInfo* info = GetAppWindowInfoForTask(app_window->task_id());
   arc::mojom::OrientationLock orientation_lock;
-  if (app_window->has_requested_orientation_lock()) {
-    orientation_lock = app_window->requested_orientation_lock();
+
+  if (info->has_requested_orientation_lock()) {
+    orientation_lock = info->requested_orientation_lock();
   } else {
     ArcAppListPrefs* prefs = ArcAppListPrefs::Get(observed_profile_);
     std::unique_ptr<ArcAppListPrefs::AppInfo> app_info =
-        prefs->GetApp(app_window->app_id());
+        prefs->GetApp(info->shelf_app_id());
     if (!app_info)
       return;
     orientation_lock = app_info->orientation_lock;
@@ -678,9 +708,8 @@ void ArcAppWindowLauncherController::SetOrientationLockForAppWindow(
   if (orientation_lock == arc::mojom::OrientationLock::CURRENT) {
     // Resolve the orientation when it first resolved.
     orientation_lock = GetCurrentOrientation();
-    app_window->set_requested_orientation_lock(orientation_lock);
+    info->set_requested_orientation_lock(orientation_lock);
   }
-
   ash::Shell* shell = ash::Shell::GetInstance();
   shell->screen_orientation_controller()->LockOrientationForWindow(
       window, BlinkOrientationLockFromMojom(orientation_lock));