Browser side changes for software compositing

content/browser/renderer_host/render_widget_host_view_aura.cc

 // Copyright (c) 2012 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 "content/browser/renderer_host/render_widget_host_view_aura.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
@@ skipping 900 matching lines @@
 
 void RenderWidgetHostViewAura::Blur() {
   window_->Blur();
 }
 
 bool RenderWidgetHostViewAura::HasFocus() const {
   return window_->HasFocus();
 }
 
 bool RenderWidgetHostViewAura::IsSurfaceAvailableForCopy() const {
-  return current_surface_ || !!host_->GetBackingStore(false);
+  return current_surface_ || current_dib_ || !!host_->GetBackingStore(false);
 }
 
 void RenderWidgetHostViewAura::Show() {
   window_->Show();
 }
 
 void RenderWidgetHostViewAura::Hide() {
   window_->Hide();
 }
 
@@ skipping 384 matching lines @@
   released_front_lock_ = NULL;
   UpdateExternalTexture();
 
   return true;
 }
 
 void RenderWidgetHostViewAura::SwapBuffersCompleted(
     const BufferPresentedCallback& ack_callback,
     const scoped_refptr<ui::Texture>& texture_to_return) {
   ui::Compositor* compositor = GetCompositor();
-  if (!compositor) {
+  if (!compositor)
     ack_callback.Run(false, texture_to_return);
-  } else {
-    // Add sending an ACK to the list of things to do OnCompositingDidCommit
-    can_lock_compositor_ = NO_PENDING_COMMIT;
-    on_compositing_did_commit_callbacks_.push_back(
-        base::Bind(ack_callback, false, texture_to_return));
-    if (!compositor->HasObserver(this))
-      compositor->AddObserver(this);
-  }
+  else
+    AddOnCommitCallback(base::Bind(ack_callback, false, texture_to_return));
 }
 
 #if defined(OS_WIN)
 void RenderWidgetHostViewAura::UpdateTransientRects(
     const std::vector<gfx::Rect>& rects) {
   transient_rects_ = rects;
   UpdateCutoutRects();
 }
 
 void RenderWidgetHostViewAura::UpdateConstrainedWindowRects(
@@ skipping 47 matching lines @@
   released_front_lock_ = NULL;
   CheckResizeLocks(frame_size_in_dip);
 
   if (paint_observer_)
     paint_observer_->OnUpdateCompositorContent();
 
   ui::Compositor* compositor = GetCompositor();
   if (!compositor) {
     SendDelegatedFrameAck();
   } else {
-    can_lock_compositor_ = NO_PENDING_COMMIT;
-    on_compositing_did_commit_callbacks_.push_back(
+    AddOnCommitCallback(
         base::Bind(&RenderWidgetHostViewAura::SendDelegatedFrameAck,
-                   base::Unretained(this)));
-    if (!compositor->HasObserver(this))
-      compositor->AddObserver(this);
+                   AsWeakPtr()));
   }
 }
 
 void RenderWidgetHostViewAura::SendDelegatedFrameAck() {
   cc::CompositorFrameAck ack;
   window_->layer()->TakeUnusedResourcesForChildCompositor(&ack.resources);
   RenderWidgetHostImpl::SendSwapCompositorFrameAck(
       host_->GetRoutingID(), host_->GetProcess()->GetID(), ack);
 }
 
+void RenderWidgetHostViewAura::SwapSoftwareFrame(
+    scoped_ptr<cc::SoftwareFrameData> frame_data,
+    float frame_device_scale_factor) {
+  const gfx::Size& frame_size = frame_data->size;
+  const gfx::Rect& damage_rect = frame_data->damage_rect;
+  const TransportDIB::Id& dib_id = frame_data->dib_id;
+
+#if defined(OS_WIN)
+  TransportDIB::Handle my_handle = TransportDIB::DefaultHandleValue();
+  ::DuplicateHandle(host_->GetProcess()->GetHandle(), dib_id.handle,
+                    ::GetCurrentProcess(), &my_handle,
+                    0, FALSE, DUPLICATE_SAME_ACCESS);
+  current_dib_.reset(TransportDIB::Map(my_handle));
+#else
+  current_dib_.reset(TransportDIB::Map(dib_id.handle));
+#endif

[piman, 2013/04/01 22:25:19]

You need to check the new dib's size (in bytes) against the frame_size (in
pixels), because it's untrusted data. Below in OnPaint you will access the
shared memory based on the frame_size, but you need to check that it's safe (if
it's not, you can discard the frame or whatever, but the renderer shouldn't be
allowed to crash - or worse - the browser).

[slavi, 2013/04/03 00:29:11]

Done.

+
+  previous_damage_.op(RectToSkIRect(damage_rect), SkRegion::kUnion_Op);
+  if (last_swapped_surface_size_ != frame_size) {
+    DLOG_IF(ERROR, damage_rect != gfx::Rect(frame_size))
+        << "Expected full damage rect";
+    last_swapped_surface_size_ = frame_size;
+  }
+
+  gfx::Size frame_size_in_dip = gfx::ToFlooredSize(
+      gfx::ScaleSize(frame_size, 1.0f / frame_device_scale_factor));
+  ui::Compositor* compositor = GetCompositor();
+  if (!compositor || !current_dib_ || ShouldSkipFrame(frame_size_in_dip)) {

[piman, 2013/04/01 22:25:19]

I don't think we can lump together those 3 cases.

For the !compositor case, we definitely want to keep the new dib anyway, but we
can return the *old* dib immediately (the old dib isn't "in flight").

For the ShouldSkipFrame flow, we can decide that we don't want to keep the new
dib around, and return it immediately, but if that's the case we need to keep
the old dib around. However, we can also keep the new dib around and return the
old dib, i.e. ignore the whole ShouldSkipFrame - it's necessary for GL textures
because they're used on another thread, but here it's only used on the main
thread so there's no race. That is, you could remove the ShouldSkipFrame logic
for the software mode.

For the !current_dib_ case, I assume this happens when we can't map for any
reason, between out-of-address-space or bogus (malicious?) data, and I guess
it's ok to drop the old dib, though you could also keep it.

[slavi, 2013/04/03 00:29:11]

I split out the check for a valid dib and dib's size.

But I still think that the ShouldSkipFrame check is necessary to make sure that
the frames that hit the screen are always the right size. And when the browser
is being resized, it waits for a frame with the correct frame to received. WDYT?

[piman, 2013/04/03 18:36:53]

ShouldSkipFrame has no impact on that though. It's there for the case where:
- the old frame is of the wrong size
- the new frame still isn't of the right size (and so we want to unblock the
renderer so that it can produce the one with the right size)
It only waits with a timeout. If we take the new frame, even though it's not the
right size, at least is more up-to-date than the old one, so that if the timeout
expires and we gutter, we have a more up-to-date frame.
Neither the old nor the new frame will be of the right size, but the new one
would be "better".

+    SendSoftwareFrameAck(dib_id);
+    return;
+  }
+
+  window_->SetExternalTexture(NULL);
+  released_front_lock_ = NULL;
+  CheckResizeLocks(frame_size_in_dip);
+  AddOnCommitCallback(
+      base::Bind(&RenderWidgetHostViewAura::SendSoftwareFrameAck,
+                 AsWeakPtr(), dib_id));

[piman, 2013/04/01 22:25:19]

I think here you need to return the old dib ID, not the new one - the browser
may need to draw multiple frames with the same renderer frame.

[slavi, 2013/04/03 00:29:11]

Done.

+
+  if (paint_observer_)
+    paint_observer_->OnUpdateCompositorContent();
+  window_->SchedulePaintInRect(ConvertRectToDIP(this, damage_rect));
+}
+
+void RenderWidgetHostViewAura::SendSoftwareFrameAck(
+    const TransportDIB::Id& id) {
+  cc::CompositorFrameAck ack;
+  ack.last_dib_id = id;
+  RenderWidgetHostImpl::SendSwapCompositorFrameAck(
+      host_->GetRoutingID(), host_->GetProcess()->GetID(), ack);
+}
+
+
 void RenderWidgetHostViewAura::OnSwapCompositorFrame(
     scoped_ptr<cc::CompositorFrame> frame) {
   if (frame->delegated_frame_data) {
     SwapDelegatedFrame(frame->delegated_frame_data.Pass(),
                        frame->metadata.device_scale_factor);
     return;
   }
+
+  if (frame->software_frame_data) {
+    SwapSoftwareFrame(frame->software_frame_data.Pass(),
+                      frame->metadata.device_scale_factor);
+    return;
+  }
+
   if (!frame->gl_frame_data || frame->gl_frame_data->mailbox.IsZero())
     return;
 
   BufferPresentedCallback ack_callback = base::Bind(
       &SendCompositorFrameAck,
       host_->GetRoutingID(), host_->GetProcess()->GetID(),
       frame->gl_frame_data->mailbox, frame->gl_frame_data->size);
 
   if (!frame->gl_frame_data->sync_point) {
     LOG(ERROR) << "CompositorFrame without sync point. Skipping frame...";
     ack_callback.Run(true, scoped_refptr<ui::Texture>());
     return;
   }
 
   ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
   factory->WaitSyncPoint(frame->gl_frame_data->sync_point);
 
   std::string mailbox_name(
       reinterpret_cast<const char*>(frame->gl_frame_data->mailbox.name),
       sizeof(frame->gl_frame_data->mailbox.name));
   BuffersSwapped(
       frame->gl_frame_data->size, mailbox_name, ack_callback);
 }
 
 void RenderWidgetHostViewAura::BuffersSwapped(
     const gfx::Size& size,
     const std::string& mailbox_name,
     const BufferPresentedCallback& ack_callback) {
   scoped_refptr<ui::Texture> texture_to_return(current_surface_);
-  const gfx::Rect surface_rect = gfx::Rect(gfx::Point(), size);
+  const gfx::Rect surface_rect = gfx::Rect(size);
   if (!SwapBuffersPrepare(
       surface_rect, surface_rect, mailbox_name, ack_callback)) {
     return;
   }
 
   previous_damage_.setRect(RectToSkIRect(surface_rect));
   skipped_damage_.setEmpty();
 
   ui::Compositor* compositor = GetCompositor();
   if (compositor) {
     gfx::Size surface_size = ConvertSizeToDIP(this, size);
     window_->SchedulePaintInRect(gfx::Rect(surface_size));
   }
 
   if (paint_observer_)
     paint_observer_->OnUpdateCompositorContent();
 
   SwapBuffersCompleted(ack_callback, texture_to_return);
 }
 
 void RenderWidgetHostViewAura::AcceleratedSurfacePostSubBuffer(
     const GpuHostMsg_AcceleratedSurfacePostSubBuffer_Params& params_in_pixel,
     int gpu_host_id) {
   scoped_refptr<ui::Texture> previous_texture(current_surface_);
   const gfx::Rect surface_rect =
-      gfx::Rect(gfx::Point(), params_in_pixel.surface_size);
+      gfx::Rect(params_in_pixel.surface_size);
   gfx::Rect damage_rect(params_in_pixel.x,
                         params_in_pixel.y,
                         params_in_pixel.width,
                         params_in_pixel.height);
   BufferPresentedCallback ack_callback = base::Bind(
       &AcknowledgeBufferForGpu, params_in_pixel.route_id, gpu_host_id,
       params_in_pixel.mailbox_name);
 
   if (!SwapBuffersPrepare(
       surface_rect, damage_rect, params_in_pixel.mailbox_name, ack_callback)) {
@@ skipping 53 matching lines @@
 void RenderWidgetHostViewAura::AcceleratedSurfaceSuspend() {
 }
 
 void RenderWidgetHostViewAura::AcceleratedSurfaceRelease() {
   // This really tells us to release the frontbuffer.
   if (current_surface_) {
     ui::Compositor* compositor = GetCompositor();
     if (compositor) {
       // We need to wait for a commit to clear to guarantee that all we
       // will not issue any more GL referencing the previous surface.
-      can_lock_compositor_ = NO_PENDING_COMMIT;
-      on_compositing_did_commit_callbacks_.push_back(
+      AddOnCommitCallback(
           base::Bind(&RenderWidgetHostViewAura::
                      SetSurfaceNotInUseByCompositor,
                      AsWeakPtr(),
                      current_surface_));  // Hold a ref so the texture will not
                                           // get deleted until after commit.
-      if (!compositor->HasObserver(this))
-        compositor->AddObserver(this);
     }
     current_surface_ = NULL;
     UpdateExternalTexture();
   }
 }
 
 bool RenderWidgetHostViewAura::HasAcceleratedSurface(
       const gfx::Size& desired_size) {
   // Aura doesn't use GetBackingStore for accelerated pages, so it doesn't
   // matter what is returned here as GetBackingStore is the only caller of this
@@ skipping 362 matching lines @@
 
 bool RenderWidgetHostViewAura::CanFocus() {
   return popup_type_ == WebKit::WebPopupTypeNone;
 }
 
 void RenderWidgetHostViewAura::OnCaptureLost() {
   host_->LostCapture();
 }
 
 void RenderWidgetHostViewAura::OnPaint(gfx::Canvas* canvas) {
-  paint_canvas_ = canvas;
-  BackingStore* backing_store = host_->GetBackingStore(true);
-  paint_canvas_ = NULL;
-  if (backing_store) {
-    static_cast<BackingStoreAura*>(backing_store)->SkiaShowRect(gfx::Point(),
-                                                                canvas);
+  bool is_compositing_active = host_->is_accelerated_compositing_active();
+  bool has_backing_store = !!host_->GetBackingStore(false);
+  if (is_compositing_active && current_dib_) {
+    SkBitmap bitmap;
+    bitmap.setConfig(SkBitmap::kARGB_8888_Config,
+                     last_swapped_surface_size_.width(),
+                     last_swapped_surface_size_.height());
+    bitmap.setPixels(current_dib_->memory());
+
+    SkCanvas* sk_canvas = canvas->sk_canvas();
+    for (SkRegion::Iterator it(previous_damage_); !it.done(); it.next()) {
+      const SkIRect& src_rect = it.rect();
+      SkRect dst_rect = SkRect::Make(src_rect);
+      sk_canvas->drawBitmapRect(bitmap, &src_rect, dst_rect, NULL);
+    }
+    previous_damage_.setEmpty();
+
+    if (paint_observer_)
+      paint_observer_->OnPaintComplete();
+  } else if (!is_compositing_active && has_backing_store) {
+    paint_canvas_ = canvas;
+    BackingStoreAura* backing_store = static_cast<BackingStoreAura*>(
+        host_->GetBackingStore(true));
+    paint_canvas_ = NULL;
+    backing_store->SkiaShowRect(gfx::Point(), canvas);
+
     if (paint_observer_)
       paint_observer_->OnPaintComplete();
   } else if (aura::Env::GetInstance()->render_white_bg()) {
     canvas->DrawColor(SK_ColorWHITE);
   }
 }
 
 void RenderWidgetHostViewAura::OnDeviceScaleFactorChanged(
     float device_scale_factor) {
   if (!host_)
@@ skipping 397 matching lines @@
 
 void RenderWidgetHostViewAura::OnCompositingDidCommit(
     ui::Compositor* compositor) {
   if (can_lock_compositor_ == NO_PENDING_COMMIT) {
     can_lock_compositor_ = YES;
     for (ResizeLockList::iterator it = resize_locks_.begin();
         it != resize_locks_.end(); ++it)
       if ((*it)->GrabDeferredLock())
         can_lock_compositor_ = YES_DID_LOCK;
   }
-  RunCompositingDidCommitCallbacks();
+  RunOnCommitCallbacks();
   locks_pending_commit_.clear();
 }
 
 void RenderWidgetHostViewAura::OnCompositingStarted(
     ui::Compositor* compositor, base::TimeTicks start_time) {
   last_draw_ended_ = start_time;
 }
 
 void RenderWidgetHostViewAura::OnCompositingEnded(
     ui::Compositor* compositor) {
@@ skipping 78 matching lines @@
 // RenderWidgetHostViewAura, ImageTransportFactoryObserver implementation:
 
 void RenderWidgetHostViewAura::OnLostResources() {
   current_surface_ = NULL;
   UpdateExternalTexture();
   locks_pending_commit_.clear();
 
   // Make sure all ImageTransportClients are deleted now that the context those
   // are using is becoming invalid. This sends pending ACKs and needs to happen
   // after calling UpdateExternalTexture() which syncs with the impl thread.
-  RunCompositingDidCommitCallbacks();
+  RunOnCommitCallbacks();
 
   DCHECK(!shared_surface_handle_.is_null());
   ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
   factory->DestroySharedSurfaceHandle(shared_surface_handle_);
   shared_surface_handle_ = factory->CreateSharedSurfaceHandle();
   host_->CompositingSurfaceUpdated();
   host_->ScheduleComposite();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ skipping 135 matching lines @@
   gfx::Rect rect = window_->bounds();
   int border_x = rect.width() * kMouseLockBorderPercentage / 100;
   int border_y = rect.height() * kMouseLockBorderPercentage / 100;
 
   return global_mouse_position_.x() < rect.x() + border_x ||
       global_mouse_position_.x() > rect.right() - border_x ||
       global_mouse_position_.y() < rect.y() + border_y ||
       global_mouse_position_.y() > rect.bottom() - border_y;
 }
 
-void RenderWidgetHostViewAura::RunCompositingDidCommitCallbacks() {
+void RenderWidgetHostViewAura::RunOnCommitCallbacks() {
   for (std::vector<base::Closure>::const_iterator
       it = on_compositing_did_commit_callbacks_.begin();
       it != on_compositing_did_commit_callbacks_.end(); ++it) {
     it->Run();
   }
   on_compositing_did_commit_callbacks_.clear();
 }
 
+void RenderWidgetHostViewAura::AddOnCommitCallback(
+    const base::Closure& callback) {
+  ui::Compositor* compositor = GetCompositor();
+  if (!compositor)

[piman, 2013/04/01 22:25:19]

In all the callsites, this has already been checked, so I would DCHECK instead.

[slavi, 2013/04/03 00:29:11]

Done.

+    return;
+
+  if (!compositor->HasObserver(this))
+    compositor->AddObserver(this);
+
+  can_lock_compositor_ = NO_PENDING_COMMIT;

[piman, 2013/04/01 22:25:19]

I feel somewhat uneasy at moving this here. I understand it saves code, but it's
mostly unrelated to the commit callback.
If you want to leave it here, maybe change the name of the method to be explicit
about this? E.g. AddOnCommitCallbackAndDisableLocks() ?

[slavi, 2013/04/03 00:29:11]

Done.

+  on_compositing_did_commit_callbacks_.push_back(callback);
+  return;

[piman, 2013/04/01 22:25:19]

not needed.

[slavi, 2013/04/03 00:29:11]

Done.

+}
+
 void RenderWidgetHostViewAura::AddedToRootWindow() {
   window_->GetRootWindow()->AddRootWindowObserver(this);
   host_->ParentChanged(GetNativeViewId());
   UpdateScreenInfo(window_);
   if (popup_type_ != WebKit::WebPopupTypeNone)
     event_filter_for_popup_exit_.reset(new EventFilterForPopupExit(this));
 }
 
 void RenderWidgetHostViewAura::RemovingFromRootWindow() {
   event_filter_for_popup_exit_.reset();
   window_->GetRootWindow()->RemoveRootWindowObserver(this);
   host_->ParentChanged(0);
   // We are about to disconnect ourselves from the compositor, we need to issue
   // the callbacks now, because we won't get notified when the frame is done.
   // TODO(piman): this might in theory cause a race where the GPU process starts
   // drawing to the buffer we haven't yet displayed. This will only show for 1
   // frame though, because we will reissue a new frame right away without that
   // composited data.
   ui::Compositor* compositor = GetCompositor();
-  RunCompositingDidCommitCallbacks();
+  RunOnCommitCallbacks();
   locks_pending_commit_.clear();
   if (compositor && compositor->HasObserver(this))
     compositor->RemoveObserver(this);
   DetachFromInputMethod();
 }
 
 ui::Compositor* RenderWidgetHostViewAura::GetCompositor() {
   aura::RootWindow* root_window = window_->GetRootWindow();
   return root_window ? root_window->compositor() : NULL;
 }
@@ skipping 12 matching lines @@
     RenderWidgetHost* widget) {
   return new RenderWidgetHostViewAura(widget);
 }
 
 // static
 void RenderWidgetHostViewPort::GetDefaultScreenInfo(WebScreenInfo* results) {
   GetScreenInfoForWindow(results, NULL);
 }
 
 }  // namespace content