Fix releasing capture in Ozone

ui/wm/core/capture_controller.cc

Index: ui/wm/core/capture_controller.cc
diff --git a/ui/wm/core/capture_controller.cc b/ui/wm/core/capture_controller.cc
index b3d8620c05e4fd907f6c74ed97b8df151014c89a..1a5c5197ee862b85e2b63fb44b3c2ad2fa7b5333 100644
--- a/ui/wm/core/capture_controller.cc
+++ b/ui/wm/core/capture_controller.cc
@@ -36,8 +36,7 @@ void CaptureController::SetCapture(aura::Window* new_capture_window) {
   DCHECK(!capture_window_ || capture_window_->GetRootWindow());
 
   aura::Window* old_capture_window = capture_window_;
-  aura::Window* old_capture_root = old_capture_window ?
-      old_capture_window->GetRootWindow() : NULL;
+  aura::client::CaptureDelegate* old_capture_delegate = capture_delegate_;
 
   // Copy the list in case it's modified out from under us.
   RootWindows root_windows(root_windows_);
@@ -54,6 +53,11 @@ void CaptureController::SetCapture(aura::Window* new_capture_window) {
   }
 
   capture_window_ = new_capture_window;
+  aura::Window* capture_root_window =
+      capture_window_ ? capture_window_->GetRootWindow() : NULL;

[sadrul, 2014/12/11 15:11:00]

nullptr

+  capture_delegate_ = capture_root_window

[sadrul, 2014/12/11 15:11:00]

Can |capture_delegate| be a local variable instead?

+      ? capture_root_window->GetHost()->dispatcher()
+      : NULL;
 
   for (RootWindows::const_iterator i = root_windows.begin();
        i != root_windows.end(); ++i) {
@@ -61,19 +65,11 @@ void CaptureController::SetCapture(aura::Window* new_capture_window) {
     delegate->UpdateCapture(old_capture_window, new_capture_window);
   }
 
-  aura::Window* capture_root =
-      capture_window_ ? capture_window_->GetRootWindow() : NULL;
-  if (capture_root != old_capture_root) {
-    if (old_capture_root) {
-      aura::client::CaptureDelegate* delegate =
-          old_capture_root->GetHost()->dispatcher();
-      delegate->ReleaseNativeCapture();
-    }
-    if (capture_root) {
-      aura::client::CaptureDelegate* delegate =
-          capture_root->GetHost()->dispatcher();
-      delegate->SetNativeCapture();
-    }
+  if (capture_delegate_ != old_capture_delegate) {
+    if (old_capture_delegate)
+      old_capture_delegate->ReleaseNativeCapture();
+    if (capture_delegate_)
+      capture_delegate_->SetNativeCapture();

[sadrul, 2014/12/11 15:11:00]

The old code makes much more sense when reading than the new code. Can you
explain how this is different from the old code?

[pkotwicz, 2014/12/11 16:22:49]

If a window with capture is reparented to a different WindowTreeHost, native
capture is NOT set on the new WindowTreeHost (or released from the old
WindowTreeHost)

When capture is released, native capture should be released from the
WindowTreeHost with capture
The old code assumed that "the WindowTreeHost with capture" is the same as the
WindowTreeHost containing |capture_window_|

In the email that I sent out on Sunday, I described why setting native capture
in WindowTreeHostOzone is unnecessary. However, we cannot remove the calls for
setting native capture here because of Windows Ash (which has bugs because it
does not set native capture)

   }
 }
 
@@ -95,7 +91,8 @@ aura::Window* CaptureController::GetGlobalCaptureWindow() {
 // CaptureController, private:
 
 CaptureController::CaptureController()
-    : capture_window_(NULL) {
+    : capture_window_(NULL),
+      capture_delegate_(NULL) {
 }
 
 CaptureController::~CaptureController() {