arc: Support Arc window showing/hiding on a user's profile switch.

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 5c2385a4bb923a683400e87492823081ea78e7b3..e4a24f963b0314b123dc376799b9d021659c170c 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
@@ -92,8 +92,9 @@ class ArcAppWindowLauncherController::AppWindow : public ui::BaseWindow {
  public:
   AppWindow(int task_id,
             const std::string app_id,
+            views::Widget* widget,

[khmel, 2016/09/14 23:42:19]

Setting widget is legacy code for one of the first arc window impl. Now it is
not required once Arc app has separate window. Moreover changing widget (and
native window) runtime is crash condition in controllers (problem with closing
windows for example).
Now it safe to discard it.

             ArcAppWindowLauncherController* owner)
-      : task_id_(task_id), app_id_(app_id), owner_(owner) {}
+      : task_id_(task_id), app_id_(app_id), widget_(widget), owner_(owner) {}
   ~AppWindow() {}
 
   void SetController(ArcAppWindowLauncherItemController* controller) {
@@ -118,16 +119,13 @@ class ArcAppWindowLauncherController::AppWindow : public ui::BaseWindow {
 
   views::Widget* widget() const { return widget_; }
 
-  void set_widget(views::Widget* widget) { widget_ = widget; }
-
   ArcAppWindowLauncherItemController* controller() { return controller_; }
 
   const std::string app_id() { return app_id_; }
 
   // ui::BaseWindow:
   bool IsActive() const override {
-    return widget_ && widget_->IsActive() &&
-           owner_->active_task_id_ == task_id_;
+    return widget_->IsActive() && owner_->active_task_id_ == task_id_;
   }
 
   bool IsMaximized() const override {
@@ -146,7 +144,7 @@ class ArcAppWindowLauncherController::AppWindow : public ui::BaseWindow {
   }
 
   gfx::NativeWindow GetNativeWindow() const override {
-    return widget_ ? widget_->GetNativeWindow() : nullptr;
+    return widget_->GetNativeWindow();
   }
 
   gfx::Rect GetRestoredBounds() const override {
@@ -164,9 +162,7 @@ class ArcAppWindowLauncherController::AppWindow : public ui::BaseWindow {
     return gfx::Rect();
   }
 
-  void Show() override {
-    // TODO(khmel): support window minimizing.
-  }
+  void Show() override { widget_->Show(); }

[khmel, 2016/09/14 23:42:19]

This is needed to correctly restore from clicking on shelf items in several
cases.

 
   void ShowInactive() override { NOTREACHED(); }
 
@@ -174,16 +170,13 @@ class ArcAppWindowLauncherController::AppWindow : public ui::BaseWindow {
 
   void Close() override { arc::CloseTask(task_id_); }
 
-  void Activate() override { arc::SetTaskActive(task_id_); }
+  void Activate() override { widget_->Activate(); }

[khmel, 2016/09/14 23:42:19]

As discussed let use wayland protocol whenever possible.

 
   void Deactivate() override { NOTREACHED(); }
 
   void Maximize() override { NOTREACHED(); }
 
-  void Minimize() override {
-    if (widget_)
-      widget_->Minimize();
-  }
+  void Minimize() override { widget_->Minimize(); }
 
   void Restore() override { NOTREACHED(); }
 
@@ -217,10 +210,10 @@ class ArcAppWindowLauncherController::AppWindow : public ui::BaseWindow {
   std::string app_id_;
   FullScreenMode fullscreen_mode_ = FullScreenMode::NOT_DEFINED;
   // Unowned pointers
+  views::Widget* widget_;

[Mr4D (OOO till 08-26), 2016/09/15 14:03:19]

could this be then const?

[khmel, 2016/09/15 15:43:27]

Indeed

   ArcAppWindowLauncherController* owner_;
   ArcAppWindowLauncherItemController* controller_ = nullptr;
   // Unowned pointer, represents host Arc window.
-  views::Widget* widget_ = nullptr;
 
   arc::mojom::OrientationLock requested_orientation_lock_ =
       arc::mojom::OrientationLock::NONE;
@@ -295,7 +288,7 @@ void ArcAppWindowLauncherController::OnWindowInitialized(aura::Window* window) {
   window->AddObserver(this);
 }
 
-void ArcAppWindowLauncherController::OnWindowVisibilityChanging(
+void ArcAppWindowLauncherController::OnWindowVisibilityChanged(

[khmel, 2016/09/14 23:42:19]

Quite tricky case. We used OnWindowVisibilityChanging event to attach controller
to window. This also registers window in multi user manager. Although window is
changing to visible, it is visible status is still off. Multi-user manager
stores its current status which is off. Of cause it sets window observer and
listen the same OnWindowVisibilityChanging. It is not called for new listener
and multi user manager thinks that Arc window is invisible. When we switch user
to primary profile back, multi user window does not show our window.

[Mr4D (OOO till 08-26), 2016/09/15 14:03:19]

You might want to add a comment here towards that. Just to make sure that a
viewer of the codes does understand why you use this and not the other.

[khmel, 2016/09/15 15:43:27]

Done.

     aura::Window* window,
     bool visible) {
   // The application id property should be set at this time.
@@ -369,8 +362,10 @@ void ArcAppWindowLauncherController::AttachControllerToWindowIfNeeded(
 
   const std::string& app_id = it->second;
 
-  std::unique_ptr<AppWindow> app_window(new AppWindow(task_id, app_id, this));
-  app_window->set_widget(views::Widget::GetWidgetForNativeWindow(window));
+  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());
   ash::SetShelfIDForWindow(app_window->shelf_id(), window);