DesktopVideoCaptureMachine: do not cache window layer pointer.

content/browser/media/capture/desktop_capture_device_aura.cc

Index: content/browser/media/capture/desktop_capture_device_aura.cc
diff --git a/content/browser/media/capture/desktop_capture_device_aura.cc b/content/browser/media/capture/desktop_capture_device_aura.cc
index 432a1854019aab49d9876a3fe646b0bb31fe9ff5..e2ece111bf593e74c43fb752721370bafa8107c4 100644
--- a/content/browser/media/capture/desktop_capture_device_aura.cc
+++ b/content/browser/media/capture/desktop_capture_device_aura.cc
@@ -149,12 +149,12 @@ class DesktopVideoCaptureMachine
   // Clears cursor state.
   void ClearCursorState();
 
+  // Helper function to return the layer associated with the desktop window.
+  ui::Layer* desktop_layer();
+
   // The window associated with the desktop.
   aura::Window* desktop_window_;
 
-  // The layer associated with the desktop.
-  ui::Layer* desktop_layer_;
-
   // The timer that kicks off period captures.
   base::Timer timer_;
 
@@ -181,7 +181,6 @@ class DesktopVideoCaptureMachine
 DesktopVideoCaptureMachine::DesktopVideoCaptureMachine(
     const DesktopMediaID& source)
     : desktop_window_(NULL),
-      desktop_layer_(NULL),
       timer_(true, true),
       window_id_(source) {}
 
@@ -197,8 +196,8 @@ bool DesktopVideoCaptureMachine::Start(
     return false;
 
   // If the desktop layer is already destroyed then return failure.
-  desktop_layer_ = desktop_window_->layer();
-  if (!desktop_layer_)
+  ui::Layer* layer = desktop_layer();
+  if (!layer)
     return false;
 
   DCHECK(oracle_proxy.get());
@@ -212,7 +211,7 @@ bool DesktopVideoCaptureMachine::Start(
   desktop_window_->AddObserver(this);
 
   // Start observing compositor updates.
-  ui::Compositor* compositor = desktop_layer_->GetCompositor();
+  ui::Compositor* compositor = layer->GetCompositor();
   if (!compositor)
     return false;
 
@@ -230,20 +229,20 @@ bool DesktopVideoCaptureMachine::Start(
 void DesktopVideoCaptureMachine::Stop(const base::Closure& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  // Stop observing compositor updates.
+  ui::Layer* layer = desktop_layer();
+  if (layer) {
+    ui::Compositor* compositor = layer->GetCompositor();
+    if (compositor)
+      compositor->RemoveObserver(this);

[piman, 2014/06/19 21:59:19]

If the layer can change, how do you know the compositor is the same as the one
in Start() ?

(well, even if it doesn't change, that may not even be true).

How about observing the window and check when it's added/removed from the tree,
and attach/detach from the compositor (that you get from the WindowTreeHost via
desktop_window_->GetHost()).

This is what we do in RenderWidgetHostViewAura.

[hshi1, 2014/06/19 22:19:33]

Done.

+  }
+
   // Stop observing window events.
   if (desktop_window_) {
     desktop_window_->RemoveObserver(this);
     desktop_window_ = NULL;
   }
 
-  // Stop observing compositor updates.
-  if (desktop_layer_) {
-    ui::Compositor* compositor = desktop_layer_->GetCompositor();
-    if (compositor)
-      compositor->RemoveObserver(this);
-    desktop_layer_ = NULL;
-  }
-
   // Stop timer.
   timer_.Stop();
 
@@ -254,9 +253,10 @@ void DesktopVideoCaptureMachine::Stop(const base::Closure& callback) {
 
 void DesktopVideoCaptureMachine::UpdateCaptureSize() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (oracle_proxy_ && desktop_layer_) {
+  ui::Layer* layer = desktop_layer();
+  if (oracle_proxy_ && layer) {
     oracle_proxy_->UpdateCaptureSize(ui::ConvertSizeToPixel(
-        desktop_layer_, desktop_layer_->bounds().size()));
+        layer, layer->bounds().size()));
   }
   ClearCursorState();
 }
@@ -265,7 +265,8 @@ void DesktopVideoCaptureMachine::Capture(bool dirty) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Do not capture if the desktop layer is already destroyed.
-  if (!desktop_layer_)
+  ui::Layer* layer = desktop_layer();
+  if (!layer)

[danakj, 2014/06/19 21:43:28]

Why are there so many checks for layer when Start() returns false if there's no
layer?

[hshi1, 2014/06/19 22:19:33]

Done.

     return;
 
   scoped_refptr<media::VideoFrame> frame;
@@ -286,7 +287,7 @@ void DesktopVideoCaptureMachine::Capture(bool dirty) {
                                gfx::Rect(desktop_window_->bounds().width(),
                                          desktop_window_->bounds().height()));
     request->set_area(window_rect);
-    desktop_layer_->RequestCopyOfOutput(request.Pass());
+    layer->RequestCopyOfOutput(request.Pass());
   }
 }
 
@@ -350,7 +351,7 @@ bool DesktopVideoCaptureMachine::ProcessCopyOutputResponse(
     const ThreadSafeCaptureOracle::CaptureFrameCallback& capture_frame_cb,
     scoped_ptr<cc::CopyOutputResult> result) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (result->IsEmpty() || result->size().IsEmpty() || !desktop_layer_)
+  if (result->IsEmpty() || result->size().IsEmpty() || !desktop_layer())
     return false;
 
   if (capture_params_.requested_format.pixel_format ==
@@ -437,7 +438,7 @@ bool DesktopVideoCaptureMachine::ProcessCopyOutputResponse(
 
 gfx::Point DesktopVideoCaptureMachine::UpdateCursorState(
     const gfx::Rect& region_in_frame) {
-  const gfx::Rect desktop_bounds = desktop_layer_->bounds();
+  const gfx::Rect desktop_bounds = desktop_layer()->bounds();
   gfx::NativeCursor cursor =
       desktop_window_->GetHost()->last_cursor();
   if (last_cursor_ != cursor) {
@@ -462,7 +463,7 @@ gfx::Point DesktopVideoCaptureMachine::UpdateCursorState(
   aura::client::GetScreenPositionClient(desktop_window_->GetRootWindow())->
       ConvertPointFromScreen(desktop_window_, &cursor_position);
   const gfx::Point hot_point_in_dip = ui::ConvertPointToDIP(
-      desktop_layer_, cursor_hot_point_);
+      desktop_layer(), cursor_hot_point_);
   cursor_position.Offset(-desktop_bounds.x() - hot_point_in_dip.x(),
                          -desktop_bounds.y() - hot_point_in_dip.y());
   return gfx::Point(
@@ -478,6 +479,12 @@ void DesktopVideoCaptureMachine::ClearCursorState() {
   scaled_cursor_bitmap_.reset();
 }
 
+ui::Layer* DesktopVideoCaptureMachine::desktop_layer() {
+  if (!desktop_window_)
+    return NULL;
+  return desktop_window_->layer();
+}
+
 void DesktopVideoCaptureMachine::OnWindowBoundsChanged(
     aura::Window* window,
     const gfx::Rect& old_bounds,