ARC IME: Fix Crash on ARC Notification

components/arc/ime/arc_ime_service.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 "components/arc/ime/arc_ime_service.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 85 matching lines @@
     ui::InputMethod* test_input_method) {
   test_input_method_ = test_input_method;
 }
 
 void ArcImeService::SetArcWindowDelegateForTesting(
     std::unique_ptr<ArcWindowDelegate> delegate) {
   arc_window_delegate_= std::move(delegate);
 }
 
 ui::InputMethod* ArcImeService::GetInputMethod() {
-  if (focused_arc_window_.windows().empty())
+  if (!focused_arc_window_)
     return nullptr;
 
   if (test_input_method_)
     return test_input_method_;
-  return focused_arc_window_.windows().front()->GetHost()->GetInputMethod();
+
+  DCHECK(focused_arc_window_->GetHost());
+  return focused_arc_window_->GetHost()->GetInputMethod();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Overridden from aura::EnvObserver:
 
 void ArcImeService::OnWindowInitialized(aura::Window* new_window) {
   if (arc_window_delegate_->IsArcWindow(new_window)) {
     if (!is_focus_observer_installed_) {
       arc_window_delegate_->RegisterFocusObserver();
       is_focus_observer_installed_ = true;
     }
   }
   keyboard::KeyboardController* keyboard_controller =
       keyboard::KeyboardController::GetInstance();
   if (keyboard_controller && keyboard_controller_ != keyboard_controller) {
     // Registering |this| as an observer only once per KeyboardController.
     keyboard_controller_ = keyboard_controller;
     keyboard_controller_->AddObserver(this);
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// Overridden from aura::WindowObserver:
+
+void ArcImeService::OnWindowDestroying(aura::Window* window) {
+  // This shouldn't be reached on production, since the window lost the focus
+  // and called OnWindowFocused() before destroying.
+  // But we handle this case for testing.
+  DCHECK_EQ(window, focused_arc_window_);
+  OnWindowFocused(nullptr, focused_arc_window_);
+}
+
+void ArcImeService::OnWindowRemovingFromRootWindow(aura::Window* window,
+                                                   aura::Window* new_root) {
+  DCHECK_EQ(window, focused_arc_window_);
+  OnWindowFocused(nullptr, focused_arc_window_);
+}
+
+////////////////////////////////////////////////////////////////////////////////
 // Overridden from exo::WMHelper::FocusChangeObserver:
 
 void ArcImeService::OnWindowFocused(aura::Window* gained_focus,
                                     aura::Window* lost_focus) {
   if (lost_focus == gained_focus)
     return;
 
-  const bool detach = (lost_focus && focused_arc_window_.Contains(lost_focus));
+  const bool detach = (lost_focus && focused_arc_window_ == lost_focus);
   const bool attach =
       (gained_focus && arc_window_delegate_->IsArcWindow(gained_focus));
 
   // TODO(kinaba): Implicit dependency in GetInputMethod as described below is
   // confusing. Consider getting InputMethod directly from lost_ or gained_focus
   // variables. For that, we need to change how to inject testing InputMethod.
   //
   // GetInputMethod() retrieves the input method associated to
   // |forcused_arc_window_|. Hence, to get the object we are detaching from, we
   // must call the method before updating the forcused ARC window.
   ui::InputMethod* const detaching_ime = detach ? GetInputMethod() : nullptr;
 
-  if (detach)
-    focused_arc_window_.Remove(lost_focus);
-  if (attach)
-    focused_arc_window_.Add(gained_focus);
+  if (detach) {
+    focused_arc_window_->RemoveObserver(this);
+    focused_arc_window_ = nullptr;
+  }
+  if (attach) {
+    DCHECK_EQ(nullptr, focused_arc_window_);
+    focused_arc_window_ = gained_focus;
+    focused_arc_window_->AddObserver(this);
+  }
 
   ui::InputMethod* const attaching_ime = attach ? GetInputMethod() : nullptr;
 
   // Notify to the input method, either when this service is detached or
   // attached. Do nothing when the focus is moving between ARC windows,
   // to avoid unpexpected context reset in ARC.
   if (detaching_ime != attaching_ime) {
     if (detaching_ime)
       detaching_ime->DetachTextInputClient(this);
     if (attaching_ime)
@@ skipping 33 matching lines @@
 void ArcImeService::ShowImeIfNeeded() {
   ui::InputMethod* const input_method = GetInputMethod();
   if (input_method && input_method->GetTextInputClient() == this) {
     input_method->ShowImeIfNeeded();
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Overridden from keyboard::KeyboardControllerObserver
 void ArcImeService::OnKeyboardBoundsChanging(const gfx::Rect& new_bounds) {
-  if (focused_arc_window_.windows().empty())
+  if (!focused_arc_window_)
     return;
-  aura::Window* window = focused_arc_window_.windows().front();
+  aura::Window* window = focused_arc_window_;
   // Multiply by the scale factor. To convert from DPI to physical pixels.
   gfx::Rect bounds_in_px = gfx::ScaleToEnclosingRect(
       new_bounds, window->layer()->device_scale_factor());

[hidehiko, 2017/02/01 04:12:35]

nit: you can inline |focused_arc_window_| here.

   ime_bridge_->SendOnKeyboardBoundsChanging(bounds_in_px);
 }
 
 void ArcImeService::OnKeyboardClosed() {}
 
 ////////////////////////////////////////////////////////////////////////////////
 // Overridden from ui::TextInputClient:
 
 void ArcImeService::SetCompositionText(
     const ui::CompositionText& composition) {
@@ skipping 55 matching lines @@
     has_composition_text_ = false;
     ime_bridge_->SendInsertText(base::string16(1, event.GetText()));
   }
 }
 
 ui::TextInputType ArcImeService::GetTextInputType() const {
   return ime_type_;
 }
 
 gfx::Rect ArcImeService::GetCaretBounds() const {
-  if (focused_arc_window_.windows().empty())
+  if (!focused_arc_window_)
     return gfx::Rect();
-  aura::Window* window = focused_arc_window_.windows().front();
+  aura::Window* window = focused_arc_window_;
 
   // |cursor_rect_| holds the rectangle reported from ARC apps, in the "screen
   // coordinates" in ARC, counted by physical pixels.
   // Chrome OS input methods expect the coordinates in Chrome OS screen, within
   // device independent pixels. Two factors are involved for the conversion.
 
   // Divide by the scale factor. To convert from physical pixels to DIP.
   gfx::Rect converted = gfx::ScaleToEnclosingRect(
       cursor_rect_, 1 / window->layer()->device_scale_factor());
 
@@ skipping 64 matching lines @@
     base::i18n::TextDirection direction) {
   return false;
 }
 
 bool ArcImeService::IsTextEditCommandEnabled(
     ui::TextEditCommand command) const {
   return false;
 }
 
 }  // namespace arc