Fix the issue that GetFrameWindow() may change between AddObserver and RemoveObserver in KeyboardCo…

ui/keyboard/keyboard_controller.cc

 // Copyright (c) 2013 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 "ui/keyboard/keyboard_controller.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_iterator.h"
@@ skipping 125 matching lines @@
   else
     command.AppendArg("0");
   VLOG(1) << "Running " << command.GetCommandLineString();
   base::LaunchOptions options;
   options.wait = true;
   base::LaunchProcess(command, options, NULL);
 #endif
 }
 
 aura::Window *GetFrameWindow(aura::Window *window) {
-  // Each container window has a non-negative id.  Stop traversing at the child
-  // of a container window.
   if (!window)
     return NULL;
-  while(window->parent() && window->parent()->id() < 0) {

[Shu Chen, 2014/09/02 04:43:42]

Kevin/Biao, do you think there is a case that a top Window had a negative id?

I've tested this change a little bit on linux_chromeos and cannot find any
problems.

If you think it'd better to be safe to check negative id here, an alternative
solution is to hold a set of "observed windows", and make sure RemoveObserver()
on all "observed windows" at destructor.

The crash bug was caused by
https://code.google.com/p/chromium/codesearch#chromium/src/ui/aura/window_obs....

[bshe, 2014/09/02 12:58:47]

I am not familiar with this code. kevers@ could comment better.

But it seems like the new code returns the top level window, while the old code
returns the immediate child of top level window if I am reading it correctly. I
am a little bit nervous about this change but I dont know enough to be certain.

[Shu Chen, 2014/09/02 14:30:43]

Done. I've changed to safer solution.

+  while(window->parent()) {
     window = window->parent();
   }
   return window;
 }
 
 }  // namespace
 
 namespace keyboard {
 
 // Observer for both keyboard show and hide animations. It should be owned by
@@ skipping 405 matching lines @@
 
 void KeyboardController::RemoveBoundsChangedObserver(aura::Window* window) {
   aura::Window* target_window = GetFrameWindow(window);
   if (target_window &&
       target_window->HasObserver(window_bounds_observer_.get())) {
     target_window->RemoveObserver(window_bounds_observer_.get());
   }
 }
 
 }  // namespace keyboard