diff options
38 files changed, 366 insertions, 280 deletions
diff --git a/chrome/browser/extensions/api/tabs/tabs_api.cc b/chrome/browser/extensions/api/tabs/tabs_api.cc index 315b72b..c70373e 100644 --- a/chrome/browser/extensions/api/tabs/tabs_api.cc +++ b/chrome/browser/extensions/api/tabs/tabs_api.cc @@ -1708,23 +1708,20 @@ bool TabsCaptureVisibleTabFunction::RunImpl() { error_ = keys::kInternalVisibleTabCaptureError; return false; } - skia::PlatformBitmap* temp_bitmap = new skia::PlatformBitmap; render_view_host->CopyFromBackingStore( gfx::Rect(), view->GetViewBounds().size(), base::Bind(&TabsCaptureVisibleTabFunction::CopyFromBackingStoreComplete, - this, - base::Owned(temp_bitmap)), - temp_bitmap); + this)); return true; } void TabsCaptureVisibleTabFunction::CopyFromBackingStoreComplete( - skia::PlatformBitmap* bitmap, - bool succeeded) { + bool succeeded, + const SkBitmap& bitmap) { if (succeeded) { VLOG(1) << "captureVisibleTab() got image from backing store."; - SendResultFromBitmap(bitmap->GetBitmap()); + SendResultFromBitmap(bitmap); return; } diff --git a/chrome/browser/extensions/api/tabs/tabs_api.h b/chrome/browser/extensions/api/tabs/tabs_api.h index 71d7d49..851aa77 100644 --- a/chrome/browser/extensions/api/tabs/tabs_api.h +++ b/chrome/browser/extensions/api/tabs/tabs_api.h @@ -37,10 +37,6 @@ struct InjectDetails; } // namespace api } // namespace extensions -namespace skia { -class PlatformBitmap; -} - // Windows class WindowsGetFunction : public SyncExtensionFunction { virtual ~WindowsGetFunction() {} @@ -201,8 +197,8 @@ class TabsCaptureVisibleTabFunction : public AsyncExtensionFunction, void SendResultFromBitmap(const SkBitmap& screen_capture); private: - void CopyFromBackingStoreComplete(skia::PlatformBitmap* bitmap, - bool succeeded); + void CopyFromBackingStoreComplete(bool succeeded, + const SkBitmap& bitmap); content::NotificationRegistrar registrar_; diff --git a/chrome/browser/prerender/prerender_tab_helper.cc b/chrome/browser/prerender/prerender_tab_helper.cc index 7139547..7950027 100644 --- a/chrome/browser/prerender/prerender_tab_helper.cc +++ b/chrome/browser/prerender/prerender_tab_helper.cc @@ -55,26 +55,23 @@ class PrerenderTabHelper::PixelStats { return; } - skia::PlatformBitmap* temp_bitmap = new skia::PlatformBitmap; web_contents->GetRenderViewHost()->CopyFromBackingStore( gfx::Rect(), gfx::Size(), base::Bind(&PrerenderTabHelper::PixelStats::HandleBitmapResult, weak_factory_.GetWeakPtr(), bitmap_type, - web_contents, - base::Owned(temp_bitmap)), - temp_bitmap); + web_contents)); } private: void HandleBitmapResult(BitmapType bitmap_type, WebContents* web_contents, - skia::PlatformBitmap* temp_bitmap, - bool succeeded) { + bool succeeded, + const SkBitmap& canvas_bitmap) { scoped_ptr<SkBitmap> bitmap; if (succeeded) { - const SkBitmap& canvas_bitmap = temp_bitmap->GetBitmap(); + // TODO(nick): This copy may now be unnecessary. bitmap.reset(new SkBitmap()); canvas_bitmap.copyTo(bitmap.get(), SkBitmap::kARGB_8888_Config); } diff --git a/chrome/browser/thumbnails/thumbnail_tab_helper.cc b/chrome/browser/thumbnails/thumbnail_tab_helper.cc index fcd71df..2ea7feb 100644 --- a/chrome/browser/thumbnails/thumbnail_tab_helper.cc +++ b/chrome/browser/thumbnails/thumbnail_tab_helper.cc @@ -16,7 +16,6 @@ #include "content/public/browser/notification_types.h" #include "content/public/browser/render_view_host.h" #include "content/public/browser/render_widget_host_view.h" -#include "skia/ext/platform_canvas.h" #include "ui/gfx/color_utils.h" #include "ui/gfx/size_conversions.h" #include "ui/gfx/screen.h" @@ -29,6 +28,8 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(ThumbnailTabHelper); +class SkBitmap; + // Overview // -------- // This class provides a service for updating thumbnails to be used in @@ -61,15 +62,14 @@ void UpdateThumbnail(const ThumbnailingContext& context, << context.score.ToString(); } -void ProcessCanvas(ThumbnailingContext* context, - ThumbnailingAlgorithm* algorithm, - skia::PlatformBitmap* temp_bitmap, - bool succeeded) { +void ProcessCapturedBitmap(ThumbnailingContext* context, + ThumbnailingAlgorithm* algorithm, + bool succeeded, + const SkBitmap& bitmap) { DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); if (!succeeded) return; - SkBitmap bitmap = temp_bitmap->GetBitmap(); algorithm->ProcessBitmap(context, base::Bind(&UpdateThumbnail), bitmap); } @@ -116,12 +116,10 @@ void AsyncProcessThumbnail(content::WebContents* web_contents, ©_rect, ©_size); - skia::PlatformBitmap* temp_bitmap = new skia::PlatformBitmap; render_widget_host->CopyFromBackingStore( copy_rect, copy_size, - base::Bind(&ProcessCanvas, context, algorithm, base::Owned(temp_bitmap)), - temp_bitmap); + base::Bind(&ProcessCapturedBitmap, context, algorithm)); } } // namespace diff --git a/content/browser/renderer_host/compositing_iosurface_mac.h b/content/browser/renderer_host/compositing_iosurface_mac.h index cd834ce..44cc639 100644 --- a/content/browser/renderer_host/compositing_iosurface_mac.h +++ b/content/browser/renderer_host/compositing_iosurface_mac.h @@ -15,6 +15,7 @@ #include "base/synchronization/lock.h" #include "base/time.h" #include "base/timer.h" +#include "third_party/skia/include/core/SkBitmap.h" #include "ui/gfx/native_widget_types.h" #include "ui/gfx/rect.h" #include "ui/gfx/size.h" @@ -58,14 +59,14 @@ class CompositingIOSurfaceMac { // into |out|. The copied region is specified with |src_pixel_subrect| and // the data is transformed so that it fits in |dst_pixel_size|. // |src_pixel_subrect| and |dst_pixel_size| are not in DIP but in pixel. - // Caller must ensure that |out| is allocated with the size no less than - // |4 * dst_pixel_size.width() * dst_pixel_size.height()| bytes. + // Caller must ensure that |out| is allocated to dimensions that match + // dst_pixel_size, with no additional padding. // |callback| is invoked when the operation is completed or failed. // Do no call this method again before |callback| is invoked. void CopyTo(const gfx::Rect& src_pixel_subrect, const gfx::Size& dst_pixel_size, - void* out, - const base::Callback<void(bool)>& callback); + const SkBitmap& out, + const base::Callback<void(bool, const SkBitmap&)>& callback); // Unref the IOSurface and delete the associated GL texture. If the GPU // process is no longer referencing it, this will delete the IOSurface. @@ -170,7 +171,7 @@ class CompositingIOSurfaceMac { pixel_buffer = 0; use_fence = false; fence = 0; - out_buf = NULL; + out_buf.reset(); callback.Reset(); } @@ -183,8 +184,8 @@ class CompositingIOSurfaceMac { GLuint fence; gfx::Rect src_rect; gfx::Size dest_size; - void* out_buf; - base::Callback<void(bool)> callback; + SkBitmap out_buf; + base::Callback<void(bool, const SkBitmap&)> callback; }; CompositingIOSurfaceMac(IOSurfaceSupport* io_surface_support, @@ -221,11 +222,12 @@ class CompositingIOSurfaceMac { // Two implementations of CopyTo() in synchronous and asynchronous mode. bool SynchronousCopyTo(const gfx::Rect& src_pixel_subrect, const gfx::Size& dst_pixel_size, - void* out); - bool AsynchronousCopyTo(const gfx::Rect& src_pixel_subrect, - const gfx::Size& dst_pixel_size, - void* out, - const base::Callback<void(bool)>& callback); + const SkBitmap& out); + bool AsynchronousCopyTo( + const gfx::Rect& src_pixel_subrect, + const gfx::Size& dst_pixel_size, + const SkBitmap& out, + const base::Callback<void(bool, const SkBitmap&)>& callback); void FinishCopy(); void CleanupResourcesForCopy(); diff --git a/content/browser/renderer_host/compositing_iosurface_mac.mm b/content/browser/renderer_host/compositing_iosurface_mac.mm index 89deb02..c22fde3 100644 --- a/content/browser/renderer_host/compositing_iosurface_mac.mm +++ b/content/browser/renderer_host/compositing_iosurface_mac.mm @@ -301,7 +301,7 @@ CompositingIOSurfaceMac::~CompositingIOSurfaceMac() { // Make sure we still run the callback if we are being destroyed with an // active copy_timer_ that has not yet fired. if (copy_context_.started) - copy_context_.callback.Run(false); + copy_context_.callback.Run(false, SkBitmap()); CVDisplayLinkRelease(display_link_); CGLSetCurrentContext(cglContext_); @@ -435,8 +435,8 @@ void CompositingIOSurfaceMac::DrawIOSurface(NSView* view, float scale_factor) { void CompositingIOSurfaceMac::CopyTo( const gfx::Rect& src_pixel_subrect, const gfx::Size& dst_pixel_size, - void* out, - const base::Callback<void(bool)>& callback) { + const SkBitmap& out, + const base::Callback<void(bool, const SkBitmap&)>& callback) { CGLSetCurrentContext(cglContext_); // Using PBO crashes on Intel drivers but not on newer Mountain Lion @@ -457,9 +457,9 @@ void CompositingIOSurfaceMac::CopyTo( if (async_copy) { if (!ret) - callback.Run(false); + callback.Run(false, SkBitmap()); } else { - callback.Run(ret); + callback.Run(ret, out); } } @@ -637,7 +637,7 @@ void CompositingIOSurfaceMac::StopDisplayLink() { bool CompositingIOSurfaceMac::SynchronousCopyTo( const gfx::Rect& src_pixel_subrect, const gfx::Size& dst_pixel_size, - void* out) { + const SkBitmap& out) { if (!MapIOSurfaceToTexture(io_surface_handle_)) return false; @@ -701,7 +701,7 @@ bool CompositingIOSurfaceMac::SynchronousCopyTo( CGLFlushDrawable(cglContext_); glReadPixels(0, 0, dst_pixel_size.width(), dst_pixel_size.height(), - GL_BGRA, GL_UNSIGNED_INT_8_8_8_8_REV, out); + GL_BGRA, GL_UNSIGNED_INT_8_8_8_8_REV, out.getPixels()); glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, 0); CHECK_GL_ERROR(); @@ -713,8 +713,8 @@ bool CompositingIOSurfaceMac::SynchronousCopyTo( bool CompositingIOSurfaceMac::AsynchronousCopyTo( const gfx::Rect& src_pixel_subrect, const gfx::Size& dst_pixel_size, - void* out, - const base::Callback<void(bool)>& callback) { + const SkBitmap& out, + const base::Callback<void(bool, const SkBitmap&)>& callback) { if (copy_context_.started) return false; @@ -840,7 +840,6 @@ void CompositingIOSurfaceMac::FinishCopy() { } } copy_timer_.Stop(); - glBindBufferARB(GL_PIXEL_PACK_BUFFER_ARB, copy_context_.pixel_buffer); CHECK_GL_ERROR(); @@ -848,16 +847,18 @@ void CompositingIOSurfaceMac::FinishCopy() { CHECK_GL_ERROR(); if (buf) { - memcpy(copy_context_.out_buf, buf, copy_context_.dest_size.GetArea() * 4); + SkAutoLockPixels bitmap_lock(copy_context_.out_buf); + memcpy(copy_context_.out_buf.getPixels(), buf, + copy_context_.dest_size.GetArea() * 4); glUnmapBufferARB(GL_PIXEL_PACK_BUFFER_ARB); CHECK_GL_ERROR(); } glBindBufferARB(GL_PIXEL_PACK_BUFFER_ARB, 0); CHECK_GL_ERROR(); - base::Callback<void(bool)> callback = copy_context_.callback; + base::Callback<void(bool, const SkBitmap&)> callback = copy_context_.callback; CleanupResourcesForCopy(); CGLSetCurrentContext(0); - callback.Run(buf != NULL); + callback.Run(buf != NULL, copy_context_.out_buf); } void CompositingIOSurfaceMac::CleanupResourcesForCopy() { diff --git a/content/browser/renderer_host/media/web_contents_video_capture_device.cc b/content/browser/renderer_host/media/web_contents_video_capture_device.cc index 0240f8b..bba926d 100644 --- a/content/browser/renderer_host/media/web_contents_video_capture_device.cc +++ b/content/browser/renderer_host/media/web_contents_video_capture_device.cc @@ -66,7 +66,6 @@ #include "media/base/bind_to_loop.h" #include "media/video/capture/video_capture_types.h" #include "skia/ext/image_operations.h" -#include "skia/ext/platform_canvas.h" #include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkColor.h" #include "ui/gfx/rect.h" @@ -147,7 +146,7 @@ class BackingStoreCopier : public WebContentsObserver { NO_SOURCE, }; typedef base::Callback<void(Result result, - scoped_ptr<skia::PlatformBitmap> capture, + const SkBitmap& capture, const base::Time& capture_time)> DoneCB; BackingStoreCopier(int render_process_id, int render_view_id); @@ -178,8 +177,9 @@ class BackingStoreCopier : public WebContentsObserver { void LookUpAndObserveWebContents(); void CopyFromBackingStoreComplete(int frame_number, - scoped_ptr<skia::PlatformBitmap> capture, - const DoneCB& done_cb, bool success); + const DoneCB& done_cb, + bool success, + const SkBitmap& result); // The "starting point" to find the capture source. const int render_process_id_; @@ -211,7 +211,7 @@ class VideoFrameRenderer { // invoke |done_cb| with a pointer to the result. The caller must guarantee // Release() will be called after the result is no longer needed. void Render(int frame_number, - scoped_ptr<skia::PlatformBitmap> capture, + const SkBitmap& capture, int frame_width, int frame_height, const DoneCB& done_cb); @@ -220,7 +220,7 @@ class VideoFrameRenderer { private: void RenderOnRenderThread(int frame_number, - scoped_ptr<skia::PlatformBitmap> capture, + const SkBitmap& capture, int frame_width, int frame_height, const DoneCB& done_cb); @@ -338,8 +338,7 @@ void BackingStoreCopier::StartCopy(int frame_number, if (!web_contents()) { // No source yet. LookUpAndObserveWebContents(); if (!web_contents()) { // No source ever. - done_cb.Run(NO_SOURCE, - scoped_ptr<skia::PlatformBitmap>(NULL), base::Time()); + done_cb.Run(NO_SOURCE, SkBitmap(), base::Time()); return; } } @@ -354,8 +353,7 @@ void BackingStoreCopier::StartCopy(int frame_number, if (!rwh) { // Transient failure state (e.g., a RenderView is being replaced). - done_cb.Run(TRANSIENT_ERROR, - scoped_ptr<skia::PlatformBitmap>(NULL), base::Time()); + done_cb.Run(TRANSIENT_ERROR, SkBitmap(), base::Time()); return; } } @@ -378,40 +376,32 @@ void BackingStoreCopier::StartCopy(int frame_number, } } - // TODO(miu): Look into tweaking the interface to CopyFromBackingStore, since - // it seems poor to have to allocate a new skia::PlatformBitmap as an output - // buffer for each successive frame (rather than reuse buffers). Perhaps - // PlatformBitmap itself should only re-Allocate when necessary? - skia::PlatformBitmap* const bitmap = new skia::PlatformBitmap(); - scoped_ptr<skia::PlatformBitmap> capture(bitmap); rwh->CopyFromBackingStore( gfx::Rect(), fitted_size, base::Bind(&BackingStoreCopier::CopyFromBackingStoreComplete, base::Unretained(this), - frame_number, base::Passed(&capture), done_cb), - bitmap); + frame_number, done_cb)); // TODO(miu): When a tab is not visible to the user, rendering stops. For // mirroring, however, it's important that rendering continues to happen. } void BackingStoreCopier::CopyFromBackingStoreComplete( - int frame_number, scoped_ptr<skia::PlatformBitmap> capture, - const DoneCB& done_cb, bool success) { + int frame_number, + const DoneCB& done_cb, + bool success, + const SkBitmap& frame) { // Note: No restriction on which thread invokes this method but, currently, // it's always the UI BrowserThread. - TRACE_EVENT_ASYNC_END1("mirroring", "Capture", this, "frame_number", frame_number); - if (success) { - done_cb.Run(OK, capture.Pass(), base::Time::Now()); + done_cb.Run(OK, frame, base::Time::Now()); } else { // Capture can fail due to transient issues, so just skip this frame. DVLOG(1) << "CopyFromBackingStore was not successful; skipping frame."; - done_cb.Run(TRANSIENT_ERROR, - scoped_ptr<skia::PlatformBitmap>(NULL), base::Time()); + done_cb.Run(TRANSIENT_ERROR, SkBitmap(), base::Time()); } } @@ -423,27 +413,26 @@ VideoFrameRenderer::VideoFrameRenderer() } void VideoFrameRenderer::Render(int frame_number, - scoped_ptr<skia::PlatformBitmap> capture, + const SkBitmap& capture, int frame_width, int frame_height, const DoneCB& done_cb) { render_thread_.message_loop()->PostTask( FROM_HERE, base::Bind(&VideoFrameRenderer::RenderOnRenderThread, base::Unretained(this), - frame_number, base::Passed(&capture), + frame_number, capture, frame_width, frame_height, done_cb)); } void VideoFrameRenderer::RenderOnRenderThread( int frame_number, - scoped_ptr<skia::PlatformBitmap> capture, + const SkBitmap& captured_bitmap, int frame_width, int frame_height, const DoneCB& done_cb) { DCHECK_EQ(render_thread_.message_loop(), MessageLoop::current()); TRACE_EVENT1("mirroring", "RenderFrame", "frame_number", frame_number); - const SkBitmap& captured_bitmap = capture->GetBitmap(); gfx::Size fitted_size; { SkAutoLockPixels locker(captured_bitmap); @@ -714,7 +703,7 @@ class CaptureMachine void SnapshotComplete(int frame_number, const base::Time& start_time, BackingStoreCopier::Result result, - scoped_ptr<skia::PlatformBitmap> capture, + const SkBitmap& capture, const base::Time& capture_time); void RenderComplete(int frame_number, const base::Time& capture_time, @@ -964,7 +953,7 @@ void CaptureMachine::StartSnapshot() { void CaptureMachine::SnapshotComplete(int frame_number, const base::Time& start_time, BackingStoreCopier::Result result, - scoped_ptr<skia::PlatformBitmap> capture, + const SkBitmap& capture, const base::Time& capture_time) { DCHECK_EQ(manager_thread_.message_loop(), MessageLoop::current()); @@ -981,11 +970,10 @@ void CaptureMachine::SnapshotComplete(int frame_number, base::Time::Now() - start_time); if (num_renders_pending_ <= 1) { ++num_renders_pending_; - DCHECK(capture); DCHECK(!capture_time.is_null()); renderer_.Render( frame_number, - capture.Pass(), + capture, settings_.width, settings_.height, media::BindToLoop(manager_thread_.message_loop_proxy(), base::Bind(&CaptureMachine::RenderComplete, this, diff --git a/content/browser/renderer_host/media/web_contents_video_capture_device_unittest.cc b/content/browser/renderer_host/media/web_contents_video_capture_device_unittest.cc index f86efc5..53b2d2f 100644 --- a/content/browser/renderer_host/media/web_contents_video_capture_device_unittest.cc +++ b/content/browser/renderer_host/media/web_contents_video_capture_device_unittest.cc @@ -44,18 +44,18 @@ class StubRenderWidgetHost : public RenderWidgetHostImpl { virtual void CopyFromBackingStore( const gfx::Rect& src_rect, const gfx::Size& accelerated_dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) OVERRIDE { - DCHECK(output); - EXPECT_TRUE(output->Allocate(kTestWidth, kTestHeight, true)); - SkBitmap bitmap = output->GetBitmap(); + const base::Callback<void(bool, const SkBitmap&)>& callback) OVERRIDE { + // Although it's not necessary, use a PlatformBitmap here (instead of a + // regular SkBitmap) to exercise possible threading issues. + scoped_ptr<skia::PlatformBitmap> platform_bitmap(new skia::PlatformBitmap); + EXPECT_TRUE(platform_bitmap->Allocate(kTestWidth, kTestHeight, false)); { - SkAutoLockPixels locker(bitmap); + SkAutoLockPixels locker(platform_bitmap->GetBitmap()); base::AutoLock guard(lock_); - bitmap.eraseColor(color_); + platform_bitmap->GetBitmap().eraseColor(color_); } - callback.Run(true); + callback.Run(true, platform_bitmap->GetBitmap()); } private: diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc index 5ad3ad4..028aa9b 100644 --- a/content/browser/renderer_host/render_widget_host_impl.cc +++ b/content/browser/renderer_host/render_widget_host_impl.cc @@ -572,8 +572,7 @@ void RenderWidgetHostImpl::SetIsLoading(bool is_loading) { void RenderWidgetHostImpl::CopyFromBackingStore( const gfx::Rect& src_subrect, const gfx::Size& accelerated_dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) { + const base::Callback<void(bool, const SkBitmap&)>& callback) { if (view_ && is_accelerated_compositing_active_) { TRACE_EVENT0("browser", "RenderWidgetHostImpl::CopyFromBackingStore::FromCompositingSurface"); @@ -581,14 +580,13 @@ void RenderWidgetHostImpl::CopyFromBackingStore( gfx::Rect(view_->GetViewBounds().size()) : src_subrect; view_->CopyFromCompositingSurface(copy_rect, accelerated_dst_size, - callback, - output); + callback); return; } BackingStore* backing_store = GetBackingStore(false); if (!backing_store) { - callback.Run(false); + callback.Run(false, SkBitmap()); return; } @@ -598,8 +596,9 @@ void RenderWidgetHostImpl::CopyFromBackingStore( gfx::Rect(backing_store->size()) : src_subrect; // When the result size is equal to the backing store size, copy from the // backing store directly to the output canvas. - bool result = backing_store->CopyFromBackingStore(copy_rect, output); - callback.Run(result); + skia::PlatformBitmap output; + bool result = backing_store->CopyFromBackingStore(copy_rect, &output); + callback.Run(result, output.GetBitmap()); } #if defined(TOOLKIT_GTK) diff --git a/content/browser/renderer_host/render_widget_host_impl.h b/content/browser/renderer_host/render_widget_host_impl.h index 8c05b49..bd228ac 100644 --- a/content/browser/renderer_host/render_widget_host_impl.h +++ b/content/browser/renderer_host/render_widget_host_impl.h @@ -109,8 +109,7 @@ class CONTENT_EXPORT RenderWidgetHostImpl : virtual public RenderWidgetHost, virtual void CopyFromBackingStore( const gfx::Rect& src_rect, const gfx::Size& accelerated_dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) OVERRIDE; + const base::Callback<void(bool, const SkBitmap&)>& callback) OVERRIDE; #if defined(TOOLKIT_GTK) virtual bool CopyFromBackingStoreToGtkWindow(const gfx::Rect& dest_rect, GdkWindow* target) OVERRIDE; diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc index b6c54b2..3bdc775 100644 --- a/content/browser/renderer_host/render_widget_host_view_android.cc +++ b/content/browser/renderer_host/render_widget_host_view_android.cc @@ -380,10 +380,9 @@ void RenderWidgetHostViewAndroid::SetBackground(const SkBitmap& background) { void RenderWidgetHostViewAndroid::CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) { + const base::Callback<void(bool, const SkBitmap&)>& callback) { NOTIMPLEMENTED(); - callback.Run(false); + callback.Run(false, SkBitmap()); } void RenderWidgetHostViewAndroid::ShowDisambiguationPopup( diff --git a/content/browser/renderer_host/render_widget_host_view_android.h b/content/browser/renderer_host/render_widget_host_view_android.h index 5ff8298..8c35d917 100644 --- a/content/browser/renderer_host/render_widget_host_view_android.h +++ b/content/browser/renderer_host/render_widget_host_view_android.h @@ -110,8 +110,7 @@ class RenderWidgetHostViewAndroid : public RenderWidgetHostViewBase { virtual void CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) OVERRIDE; + const base::Callback<void(bool, const SkBitmap&)>& callback) OVERRIDE; virtual BackingStore* AllocBackingStore(const gfx::Size& size) OVERRIDE; virtual gfx::GLSurfaceHandle GetCompositingSurface() OVERRIDE; virtual void GetScreenInfo(WebKit::WebScreenInfo* results) OVERRIDE; diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc index 914b155..7abf0ef 100644 --- a/content/browser/renderer_host/render_widget_host_view_aura.cc +++ b/content/browser/renderer_host/render_widget_host_view_aura.cc @@ -711,31 +711,35 @@ BackingStore* RenderWidgetHostViewAura::AllocBackingStore( void RenderWidgetHostViewAura::CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) { - base::ScopedClosureRunner scoped_callback_runner(base::Bind(callback, false)); + const base::Callback<void(bool, const SkBitmap&)>& callback) { + base::ScopedClosureRunner scoped_callback_runner( + base::Bind(callback, false, SkBitmap())); if (!current_surface_) return; gfx::Size dst_size_in_pixel = ConvertSizeToPixel(this, dst_size); - if (!output->Allocate( - dst_size_in_pixel.width(), dst_size_in_pixel.height(), true)) + + SkBitmap output; + output.setConfig(SkBitmap::kARGB_8888_Config, + dst_size_in_pixel.width(), dst_size_in_pixel.height()); + if (!output.allocPixels()) return; + output.setIsOpaque(true); ImageTransportFactory* factory = ImageTransportFactory::GetInstance(); GLHelper* gl_helper = factory->GetGLHelper(); if (!gl_helper) return; - unsigned char* addr = static_cast<unsigned char*>( - output->GetBitmap().getPixels()); + unsigned char* addr = static_cast<unsigned char*>(output.getPixels()); scoped_callback_runner.Release(); // Wrap the callback with an internal handler so that we can inject our // own completion handlers (where we can try to free the frontbuffer). base::Callback<void(bool)> wrapper_callback = base::Bind( &RenderWidgetHostViewAura::CopyFromCompositingSurfaceFinished, AsWeakPtr(), + output, callback); ++pending_thumbnail_tasks_; @@ -1012,9 +1016,10 @@ void RenderWidgetHostViewAura::SetSurfaceNotInUseByCompositor( void RenderWidgetHostViewAura::CopyFromCompositingSurfaceFinished( base::WeakPtr<RenderWidgetHostViewAura> render_widget_host_view, - const base::Callback<void(bool)>& callback, + const SkBitmap& bitmap, + const base::Callback<void(bool, const SkBitmap&)>& callback, bool result) { - callback.Run(result); + callback.Run(result, bitmap); if (!render_widget_host_view.get()) return; diff --git a/content/browser/renderer_host/render_widget_host_view_aura.h b/content/browser/renderer_host/render_widget_host_view_aura.h index e2f27a9..f47e110 100644 --- a/content/browser/renderer_host/render_widget_host_view_aura.h +++ b/content/browser/renderer_host/render_widget_host_view_aura.h @@ -146,8 +146,7 @@ class RenderWidgetHostViewAura virtual void CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) OVERRIDE; + const base::Callback<void(bool, const SkBitmap&)>& callback) OVERRIDE; virtual void OnAcceleratedCompositingStateChange() OVERRIDE; virtual void AcceleratedSurfaceBuffersSwapped( const GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params& params_in_pixel, @@ -325,7 +324,8 @@ class RenderWidgetHostViewAura // AdjustSurfaceProtection. static void CopyFromCompositingSurfaceFinished( base::WeakPtr<RenderWidgetHostViewAura> render_widget_host_view, - const base::Callback<void(bool)>& callback, + const SkBitmap& bitmap, + const base::Callback<void(bool, const SkBitmap&)>& callback, bool result); ui::Compositor* GetCompositor(); diff --git a/content/browser/renderer_host/render_widget_host_view_browsertest.cc b/content/browser/renderer_host/render_widget_host_view_browsertest.cc index 0a32b98..066ba35 100644 --- a/content/browser/renderer_host/render_widget_host_view_browsertest.cc +++ b/content/browser/renderer_host/render_widget_host_view_browsertest.cc @@ -33,7 +33,8 @@ class RenderWidgetHostViewBrowserTest : public ContentBrowserTest { ui::DisableTestCompositor(); } - void FinishCopyFromBackingStore(bool expected_result, bool result) { + void FinishCopyFromBackingStore(bool expected_result, bool result, + const SkBitmap& bitmap) { ASSERT_EQ(expected_result, result); finish_called_ = true; } @@ -73,13 +74,11 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewBrowserTest, ASSERT_LT(increment, 50); } - skia::PlatformBitmap bitmap; rwh->CopyFromBackingStore( gfx::Rect(), size, base::Bind(&RenderWidgetHostViewBrowserTest::FinishCopyFromBackingStore, - base::Unretained(this), false), - &bitmap); + base::Unretained(this), false)); // Delete the surface before the callback is run. This is synchronous until // we get to the copy_timer_, so we will always end up in the destructor diff --git a/content/browser/renderer_host/render_widget_host_view_gtk.cc b/content/browser/renderer_host/render_widget_host_view_gtk.cc index 04396df..132f8b8 100644 --- a/content/browser/renderer_host/render_widget_host_view_gtk.cc +++ b/content/browser/renderer_host/render_widget_host_view_gtk.cc @@ -1020,9 +1020,9 @@ BackingStore* RenderWidgetHostViewGtk::AllocBackingStore( void RenderWidgetHostViewGtk::CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& /* dst_size */, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) { - base::ScopedClosureRunner scoped_callback_runner(base::Bind(callback, false)); + const base::Callback<void(bool, const SkBitmap&)>& callback) { + base::ScopedClosureRunner scoped_callback_runner( + base::Bind(callback, false, SkBitmap())); gfx::Rect src_subrect_in_view = src_subrect; src_subrect_in_view.Offset(GetViewBounds().OffsetFromOrigin()); @@ -1036,10 +1036,15 @@ void RenderWidgetHostViewGtk::CopyFromCompositingSurface( if (!image.get()) return; - if (!output->Allocate(src_subrect.width(), src_subrect.height(), true)) + SkBitmap bitmap; + bitmap.setConfig(SkBitmap::kARGB_8888_Config, + image->width, + image->height, + image->bytes_per_line); + if (!bitmap.allocPixels()) return; + bitmap.setIsOpaque(true); - const SkBitmap& bitmap = output->GetBitmap(); const size_t bitmap_size = bitmap.getSize(); DCHECK_EQ(bitmap_size, static_cast<size_t>(image->height * image->bytes_per_line)); @@ -1047,7 +1052,7 @@ void RenderWidgetHostViewGtk::CopyFromCompositingSurface( memcpy(pixels, image->data, bitmap_size); scoped_callback_runner.Release(); - callback.Run(true); + callback.Run(true, bitmap); } void RenderWidgetHostViewGtk::AcceleratedSurfaceBuffersSwapped( diff --git a/content/browser/renderer_host/render_widget_host_view_gtk.h b/content/browser/renderer_host/render_widget_host_view_gtk.h index 4b5acf0..7eaa107 100644 --- a/content/browser/renderer_host/render_widget_host_view_gtk.h +++ b/content/browser/renderer_host/render_widget_host_view_gtk.h @@ -103,8 +103,7 @@ class CONTENT_EXPORT RenderWidgetHostViewGtk virtual void CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) OVERRIDE; + const base::Callback<void(bool, const SkBitmap&)>& callback) OVERRIDE; virtual void OnAcceleratedCompositingStateChange() OVERRIDE; virtual void AcceleratedSurfaceBuffersSwapped( const GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params& params, diff --git a/content/browser/renderer_host/render_widget_host_view_guest.cc b/content/browser/renderer_host/render_widget_host_view_guest.cc index 8f0cd12..0039df7 100644 --- a/content/browser/renderer_host/render_widget_host_view_guest.cc +++ b/content/browser/renderer_host/render_widget_host_view_guest.cc @@ -208,8 +208,8 @@ BackingStore* RenderWidgetHostViewGuest::AllocBackingStore( void RenderWidgetHostViewGuest::CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& /* dst_size */, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) { + const base::Callback<void(bool, const SkBitmap&)>& callback) { + callback.Run(false, SkBitmap()); } void RenderWidgetHostViewGuest::AcceleratedSurfaceSuspend() { diff --git a/content/browser/renderer_host/render_widget_host_view_guest.h b/content/browser/renderer_host/render_widget_host_view_guest.h index f705b78..c84a2ae 100644 --- a/content/browser/renderer_host/render_widget_host_view_guest.h +++ b/content/browser/renderer_host/render_widget_host_view_guest.h @@ -91,8 +91,7 @@ class CONTENT_EXPORT RenderWidgetHostViewGuest virtual void CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) OVERRIDE; + const base::Callback<void(bool, const SkBitmap&)>& callback) OVERRIDE; virtual void OnAcceleratedCompositingStateChange() OVERRIDE; virtual void AcceleratedSurfaceBuffersSwapped( const GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params& params, diff --git a/content/browser/renderer_host/render_widget_host_view_mac.h b/content/browser/renderer_host/render_widget_host_view_mac.h index 894673b..88200bd 100644 --- a/content/browser/renderer_host/render_widget_host_view_mac.h +++ b/content/browser/renderer_host/render_widget_host_view_mac.h @@ -267,8 +267,7 @@ class RenderWidgetHostViewMac : public RenderWidgetHostViewBase { virtual void CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) OVERRIDE; + const base::Callback<void(bool, const SkBitmap&)>& callback) OVERRIDE; virtual void OnAcceleratedCompositingStateChange() OVERRIDE; virtual void OnAccessibilityNotifications( diff --git a/content/browser/renderer_host/render_widget_host_view_mac.mm b/content/browser/renderer_host/render_widget_host_view_mac.mm index dff78cb..7d35517 100644 --- a/content/browser/renderer_host/render_widget_host_view_mac.mm +++ b/content/browser/renderer_host/render_widget_host_view_mac.mm @@ -935,9 +935,9 @@ BackingStore* RenderWidgetHostViewMac::AllocBackingStore( void RenderWidgetHostViewMac::CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) { - base::ScopedClosureRunner scoped_callback_runner(base::Bind(callback, false)); + const base::Callback<void(bool, const SkBitmap&)>& callback) { + base::ScopedClosureRunner scoped_callback_runner( + base::Bind(callback, false, SkBitmap())); if (!compositing_iosurface_.get() || !compositing_iosurface_->HasIOSurface()) return; @@ -945,9 +945,14 @@ void RenderWidgetHostViewMac::CopyFromCompositingSurface( float scale = ScaleFactor(cocoa_view_); gfx::Size dst_pixel_size = gfx::ToFlooredSize( gfx::ScaleSize(dst_size, scale)); - if (!output->Allocate( - dst_pixel_size.width(), dst_pixel_size.height(), true)) + + SkBitmap output; + output.setConfig(SkBitmap::kARGB_8888_Config, + dst_pixel_size.width(), dst_pixel_size.height()); + if (!output.allocPixels()) return; + output.setIsOpaque(true); + scoped_callback_runner.Release(); // Convert |src_subrect| from the views coordinate (upper-left origin) into @@ -960,7 +965,7 @@ void RenderWidgetHostViewMac::CopyFromCompositingSurface( compositing_iosurface_->CopyTo( src_pixel_gl_subrect, dst_pixel_size, - output->GetBitmap().getPixels(), + output, callback); } diff --git a/content/browser/renderer_host/render_widget_host_view_win.cc b/content/browser/renderer_host/render_widget_host_view_win.cc index cc7b84b..0e3e081 100644 --- a/content/browser/renderer_host/render_widget_host_view_win.cc +++ b/content/browser/renderer_host/render_widget_host_view_win.cc @@ -816,23 +816,19 @@ BackingStore* RenderWidgetHostViewWin::AllocBackingStore( void RenderWidgetHostViewWin::CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) { - base::ScopedClosureRunner scoped_callback_runner(base::Bind(callback, false)); + const base::Callback<void(bool, const SkBitmap&)>& callback) { + base::ScopedClosureRunner scoped_callback_runner( + base::Bind(callback, false, SkBitmap())); if (!accelerated_surface_.get()) return; if (dst_size.IsEmpty()) return; - if (!output->Allocate(dst_size.width(), dst_size.height(), true)) - return; - scoped_callback_runner.Release(); accelerated_surface_->AsyncCopyTo( src_subrect, dst_size, - output->GetBitmap().getPixels(), callback); } diff --git a/content/browser/renderer_host/render_widget_host_view_win.h b/content/browser/renderer_host/render_widget_host_view_win.h index d70f355..357b845 100644 --- a/content/browser/renderer_host/render_widget_host_view_win.h +++ b/content/browser/renderer_host/render_widget_host_view_win.h @@ -205,8 +205,7 @@ class RenderWidgetHostViewWin virtual void CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) OVERRIDE; + const base::Callback<void(bool, const SkBitmap&)>& callback) OVERRIDE; virtual void OnAcceleratedCompositingStateChange() OVERRIDE; virtual void ProcessAckedTouchEvent(const WebKit::WebTouchEvent& touch, InputEventAckState ack_result) OVERRIDE; diff --git a/content/browser/renderer_host/test_render_view_host.cc b/content/browser/renderer_host/test_render_view_host.cc index 6bea59f..3b34061 100644 --- a/content/browser/renderer_host/test_render_view_host.cc +++ b/content/browser/renderer_host/test_render_view_host.cc @@ -118,9 +118,8 @@ BackingStore* TestRenderWidgetHostView::AllocBackingStore( void TestRenderWidgetHostView::CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) { - callback.Run(false); + const base::Callback<void(bool, const SkBitmap&)>& callback) { + callback.Run(false, SkBitmap()); } void TestRenderWidgetHostView::OnAcceleratedCompositingStateChange() { diff --git a/content/browser/renderer_host/test_render_view_host.h b/content/browser/renderer_host/test_render_view_host.h index ceab8e5..f5b9632 100644 --- a/content/browser/renderer_host/test_render_view_host.h +++ b/content/browser/renderer_host/test_render_view_host.h @@ -111,8 +111,7 @@ class TestRenderWidgetHostView : public RenderWidgetHostViewBase { virtual void CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) OVERRIDE; + const base::Callback<void(bool, const SkBitmap&)>& callback) OVERRIDE; virtual void OnAcceleratedCompositingStateChange() OVERRIDE; virtual void AcceleratedSurfaceBuffersSwapped( const GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params& params, diff --git a/content/browser/web_contents/navigation_controller_impl.cc b/content/browser/web_contents/navigation_controller_impl.cc index a4ec35f..3d74699 100644 --- a/content/browser/web_contents/navigation_controller_impl.cc +++ b/content/browser/web_contents/navigation_controller_impl.cc @@ -501,20 +501,17 @@ void NavigationControllerImpl::TakeScreenshot() { if (!take_screenshot_callback_.is_null()) take_screenshot_callback_.Run(render_view_host); - skia::PlatformBitmap* temp_bitmap = new skia::PlatformBitmap; render_view_host->CopyFromBackingStore(gfx::Rect(), view->GetViewBounds().size(), base::Bind(&NavigationControllerImpl::OnScreenshotTaken, base::Unretained(this), - entry->GetUniqueID(), - base::Owned(temp_bitmap)), - temp_bitmap); + entry->GetUniqueID())); } void NavigationControllerImpl::OnScreenshotTaken( int unique_id, - skia::PlatformBitmap* bitmap, - bool success) { + bool success, + const SkBitmap& bitmap) { NavigationEntryImpl* entry = NULL; for (NavigationEntries::iterator i = entries_.begin(); i != entries_.end(); @@ -536,7 +533,7 @@ void NavigationControllerImpl::OnScreenshotTaken( } std::vector<unsigned char> data; - if (gfx::PNGCodec::EncodeBGRASkBitmap(bitmap->GetBitmap(), true, &data)) { + if (gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, true, &data)) { if (!entry->screenshot()) ++screenshot_count_; entry->SetScreenshotPNGData(data); diff --git a/content/browser/web_contents/navigation_controller_impl.h b/content/browser/web_contents/navigation_controller_impl.h index 6661120..4b86829 100644 --- a/content/browser/web_contents/navigation_controller_impl.h +++ b/content/browser/web_contents/navigation_controller_impl.h @@ -15,12 +15,9 @@ #include "content/public/browser/navigation_controller.h" #include "content/public/browser/navigation_type.h" +class SkBitmap; struct ViewHostMsg_FrameNavigate_Params; -namespace skia { -class PlatformBitmap; -} - namespace content { class NavigationEntryImpl; class RenderViewHost; @@ -325,8 +322,8 @@ class CONTENT_EXPORT NavigationControllerImpl // The callback invoked when taking the screenshot of the page is complete. // This sets the screenshot on the navigation entry. void OnScreenshotTaken(int unique_id, - skia::PlatformBitmap* bitmap, - bool success); + bool success, + const SkBitmap& bitmap); // Removes the screenshot for the entry (and updates the count as // appropriate). diff --git a/content/browser/web_contents/navigation_controller_impl_unittest.cc b/content/browser/web_contents/navigation_controller_impl_unittest.cc index 2a4592b..8e527a7 100644 --- a/content/browser/web_contents/navigation_controller_impl_unittest.cc +++ b/content/browser/web_contents/navigation_controller_impl_unittest.cc @@ -3030,8 +3030,10 @@ TEST_F(NavigationControllerTest, MAYBE_PurgeScreenshot) { NavigationControllerImpl& controller = controller_impl(); // Prepare some data to use as screenshot for each navigation. - skia::PlatformBitmap bitmap; - ASSERT_TRUE(bitmap.Allocate(1, 1, false)); + SkBitmap bitmap; + bitmap.setConfig(SkBitmap::kARGB_8888_Config, 1, 1); + ASSERT_TRUE(bitmap.allocPixels()); + bitmap.setIsOpaque(false); NavigationEntryImpl* entry; // Navigate enough times to make sure that some screenshots are purged. @@ -3044,7 +3046,7 @@ TEST_F(NavigationControllerTest, MAYBE_PurgeScreenshot) { for (int i = 0; i < controller.GetEntryCount(); ++i) { entry = NavigationEntryImpl::FromNavigationEntry( controller.GetEntryAtIndex(i)); - controller.OnScreenshotTaken(entry->GetUniqueID(), &bitmap, true); + controller.OnScreenshotTaken(entry->GetUniqueID(), true, bitmap); EXPECT_TRUE(entry->screenshot()); } @@ -3052,7 +3054,7 @@ TEST_F(NavigationControllerTest, MAYBE_PurgeScreenshot) { EXPECT_EQ(13, controller.GetEntryCount()); entry = NavigationEntryImpl::FromNavigationEntry( controller.GetEntryAtIndex(11)); - controller.OnScreenshotTaken(entry->GetUniqueID(), &bitmap, true); + controller.OnScreenshotTaken(entry->GetUniqueID(), true, bitmap); for (int i = 0; i < 2; ++i) { entry = NavigationEntryImpl::FromNavigationEntry( @@ -3073,14 +3075,14 @@ TEST_F(NavigationControllerTest, MAYBE_PurgeScreenshot) { for (int i = 0; i < controller.GetEntryCount() - 1; ++i) { entry = NavigationEntryImpl::FromNavigationEntry( controller.GetEntryAtIndex(i)); - controller.OnScreenshotTaken(entry->GetUniqueID(), &bitmap, true); + controller.OnScreenshotTaken(entry->GetUniqueID(), true, bitmap); } for (int i = 10; i <= 12; ++i) { entry = NavigationEntryImpl::FromNavigationEntry( controller.GetEntryAtIndex(i)); EXPECT_FALSE(entry->screenshot()) << "Screenshot " << i << " not purged"; - controller.OnScreenshotTaken(entry->GetUniqueID(), &bitmap, true); + controller.OnScreenshotTaken(entry->GetUniqueID(), true, bitmap); } // Navigate to index 7 and assign screenshot to all entries. @@ -3090,7 +3092,7 @@ TEST_F(NavigationControllerTest, MAYBE_PurgeScreenshot) { for (int i = 0; i < controller.GetEntryCount() - 1; ++i) { entry = NavigationEntryImpl::FromNavigationEntry( controller.GetEntryAtIndex(i)); - controller.OnScreenshotTaken(entry->GetUniqueID(), &bitmap, true); + controller.OnScreenshotTaken(entry->GetUniqueID(), true, bitmap); } for (int i = 0; i < 2; ++i) { diff --git a/content/port/browser/render_widget_host_view_port.h b/content/port/browser/render_widget_host_view_port.h index 70d6b7d..953296c 100644 --- a/content/port/browser/render_widget_host_view_port.h +++ b/content/port/browser/render_widget_host_view_port.h @@ -17,6 +17,7 @@ #include "ui/base/range/range.h" #include "ui/surface/transport_dib.h" +class SkBitmap; class WebCursor; struct AccessibilityHostMsg_NotificationParams; @@ -34,10 +35,6 @@ namespace WebKit { struct WebScreenInfo; } -namespace skia { -class PlatformBitmap; -}; - namespace content { class BackingStore; class SmoothScrollGesture; @@ -164,8 +161,7 @@ class CONTENT_EXPORT RenderWidgetHostViewPort : public RenderWidgetHostView { virtual void CopyFromCompositingSurface( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) = 0; + const base::Callback<void(bool, const SkBitmap&)>& callback) = 0; // Called when accelerated compositing state changes. virtual void OnAcceleratedCompositingStateChange() = 0; diff --git a/content/public/browser/render_widget_host.h b/content/public/browser/render_widget_host.h index 033b91c..a0e8d8a 100644 --- a/content/public/browser/render_widget_host.h +++ b/content/public/browser/render_widget_host.h @@ -22,14 +22,12 @@ #include "skia/ext/platform_device.h" #endif +class SkBitmap; + namespace gfx { class Rect; } -namespace skia { -class PlatformBitmap; -} - namespace content { class RenderProcessHost; @@ -170,22 +168,24 @@ class CONTENT_EXPORT RenderWidgetHost : public IPC::Sender { // contents, but e.g. in the omnibox. virtual void SetActive(bool active) = 0; - // Copies the given subset of the backing store into the given (uninitialized) - // PlatformCanvas. If |src_rect| is empty, the whole contents is copied. - // If non empty |accelerated_dest_size| is given and accelerated compositing - // is active, the content is shrinked so that it fits in - // |accelerated_dest_size|. If |accelerated_dest_size| is larger than the - // contens size, the content is not resized. If |accelerated_dest_size| is - // empty, the size copied from the source contents is used. - // |callback| is invoked with true on success, false otherwise. |output| can - // be initialized even on failure. + // Copies the given subset of the backing store, and passes the result as a + // bitmap to a callback. + // + // If |src_rect| is empty, the whole contents is copied. If non empty + // |accelerated_dst_size| is given and accelerated compositing is active, the + // content is shrunk so that it fits in |accelerated_dst_size|. If + // |accelerated_dst_size| is larger than the content size, the content is not + // resized. If |accelerated_dst_size| is empty, the size copied from the + // source contents is used. |callback| is invoked with true on success, false + // otherwise, along with a SkBitmap containing the copied pixel data. + // // NOTE: |callback| is called synchronously if the backing store is available. - // When accelerated compositing is active, it is called asynchronously on Aura - // and synchronously on the other platforms. - virtual void CopyFromBackingStore(const gfx::Rect& src_rect, - const gfx::Size& accelerated_dest_size, - const base::Callback<void(bool)>& callback, - skia::PlatformBitmap* output) = 0; + // When accelerated compositing is active, |callback| may be called + // asynchronously. + virtual void CopyFromBackingStore( + const gfx::Rect& src_rect, + const gfx::Size& accelerated_dst_size, + const base::Callback<void(bool, const SkBitmap&)>& callback) = 0; #if defined(TOOLKIT_GTK) // Paint the backing store into the target's |dest_rect|. virtual bool CopyFromBackingStoreToGtkWindow(const gfx::Rect& dest_rect, diff --git a/skia/ext/bitmap_platform_device_android.cc b/skia/ext/bitmap_platform_device_android.cc index 3a043a8..8a29586 100644 --- a/skia/ext/bitmap_platform_device_android.cc +++ b/skia/ext/bitmap_platform_device_android.cc @@ -82,8 +82,9 @@ SkCanvas* CreatePlatformCanvas(int width, int height, bool is_opaque, } // Port of PlatformBitmap to android - -PlatformBitmap::~PlatformBitmap() {} +PlatformBitmap::~PlatformBitmap() { + // Nothing to do. +} bool PlatformBitmap::Allocate(int width, int height, bool is_opaque) { bitmap_.setConfig(SkBitmap::kARGB_8888_Config, width, height); diff --git a/skia/ext/bitmap_platform_device_linux.cc b/skia/ext/bitmap_platform_device_linux.cc index a35ba77..c9a0214 100644 --- a/skia/ext/bitmap_platform_device_linux.cc +++ b/skia/ext/bitmap_platform_device_linux.cc @@ -186,27 +186,34 @@ SkCanvas* CreatePlatformCanvas(int width, int height, bool is_opaque, } // Port of PlatformBitmap to linux - PlatformBitmap::~PlatformBitmap() { cairo_destroy(surface_); } bool PlatformBitmap::Allocate(int width, int height, bool is_opaque) { - cairo_surface_t* surf = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, - width, height); + // The SkBitmap allocates and owns the bitmap memory; PlatformBitmap owns the + // cairo drawing context tied to the bitmap. The SkBitmap's pixelRef can + // outlive the PlatformBitmap if additional copies are made. + int stride = cairo_format_stride_for_width(CAIRO_FORMAT_ARGB32, width); + bitmap_.setConfig(SkBitmap::kARGB_8888_Config, width, height, stride); + if (!bitmap_.allocPixels()) // Using the default allocator. + return false; + bitmap_.setIsOpaque(is_opaque); + + cairo_surface_t* surf = cairo_image_surface_create_for_data( + reinterpret_cast<unsigned char*>(bitmap_.getPixels()), + CAIRO_FORMAT_ARGB32, + width, + height, + stride); if (cairo_surface_status(surf) != CAIRO_STATUS_SUCCESS) { cairo_surface_destroy(surf); return false; } - bitmap_.setConfig(SkBitmap::kARGB_8888_Config, width, height, - cairo_image_surface_get_stride(surf)); - bitmap_.setPixels(cairo_image_surface_get_data(surf)); - bitmap_.setIsOpaque(is_opaque); - surface_ = cairo_create(surf); cairo_surface_destroy(surf); return true; } - + } // namespace skia diff --git a/skia/ext/bitmap_platform_device_win.cc b/skia/ext/bitmap_platform_device_win.cc index 8c304e6..201866a 100644 --- a/skia/ext/bitmap_platform_device_win.cc +++ b/skia/ext/bitmap_platform_device_win.cc @@ -13,15 +13,35 @@ #include "third_party/skia/include/core/SkRegion.h" #include "third_party/skia/include/core/SkUtils.h" -static HBITMAP CreateHBitmap(int width, int height, bool is_opaque, - HANDLE shared_section, SkBitmap* output) { +namespace { + +// PlatformBitmapPixelRef is an SkPixelRef that, on Windows, is backed by an +// HBITMAP. +class SK_API PlatformBitmapPixelRef : public SkPixelRef { + public: + PlatformBitmapPixelRef(HBITMAP bitmap_handle, void* pixels); + virtual ~PlatformBitmapPixelRef(); + + SK_DECLARE_UNFLATTENABLE_OBJECT(); + + protected: + virtual void* onLockPixels(SkColorTable**) SK_OVERRIDE; + virtual void onUnlockPixels() SK_OVERRIDE; + + private: + HBITMAP bitmap_handle_; + void* pixels_; +}; + +HBITMAP CreateHBitmap(int width, int height, bool is_opaque, + HANDLE shared_section, void** data) { // CreateDIBSection appears to get unhappy if we create an empty bitmap, so // just create a minimal bitmap if ((width == 0) || (height == 0)) { width = 1; height = 1; } - + BITMAPINFOHEADER hdr = {0}; hdr.biSize = sizeof(BITMAPINFOHEADER); hdr.biWidth = width; @@ -35,17 +55,35 @@ static HBITMAP CreateHBitmap(int width, int height, bool is_opaque, hdr.biClrUsed = 0; hdr.biClrImportant = 0; - void* data = NULL; HBITMAP hbitmap = CreateDIBSection(NULL, reinterpret_cast<BITMAPINFO*>(&hdr), - 0, &data, shared_section, 0); - if (hbitmap) { - output->setConfig(SkBitmap::kARGB_8888_Config, width, height); - output->setPixels(data); - output->setIsOpaque(is_opaque); - } + 0, data, shared_section, 0); return hbitmap; } +PlatformBitmapPixelRef::PlatformBitmapPixelRef(HBITMAP bitmap_handle, + void* pixels) + : bitmap_handle_(bitmap_handle), + pixels_(pixels) { + setPreLocked(pixels, NULL); +} + +PlatformBitmapPixelRef::~PlatformBitmapPixelRef() { + if (bitmap_handle_) + DeleteObject(bitmap_handle_); +} + +void* PlatformBitmapPixelRef::onLockPixels(SkColorTable** color_table) { + *color_table = NULL; + return pixels_; +} + +void PlatformBitmapPixelRef::onUnlockPixels() { + // Nothing to do. + return; +} + +} // namespace + namespace skia { BitmapPlatformDevice::BitmapPlatformDeviceData::BitmapPlatformDeviceData( @@ -125,13 +163,18 @@ BitmapPlatformDevice* BitmapPlatformDevice::Create( int height, bool is_opaque, HANDLE shared_section) { - SkBitmap bitmap; + void* data; HBITMAP hbitmap = CreateHBitmap(width, height, is_opaque, shared_section, - &bitmap); + &data); if (!hbitmap) return NULL; + SkBitmap bitmap; + bitmap.setConfig(SkBitmap::kARGB_8888_Config, width, height); + bitmap.setPixels(data); + bitmap.setIsOpaque(is_opaque); + #ifndef NDEBUG // If we were given data, then don't clobber it! if (!shared_section && is_opaque) @@ -279,28 +322,34 @@ SkCanvas* CreatePlatformCanvas(int width, // Port of PlatformBitmap to win PlatformBitmap::~PlatformBitmap() { - if (surface_) + if (surface_) { + if (platform_extra_) + SelectObject(surface_, reinterpret_cast<HGDIOBJ>(platform_extra_)); DeleteDC(surface_); - - HBITMAP hbitmap = (HBITMAP)platform_extra_; - if (hbitmap) - DeleteObject(hbitmap); + } } bool PlatformBitmap::Allocate(int width, int height, bool is_opaque) { - HBITMAP hbitmap = CreateHBitmap(width, height, is_opaque, 0, &bitmap_); + void* data; + HBITMAP hbitmap = CreateHBitmap(width, height, is_opaque, 0, &data); if (!hbitmap) return false; surface_ = CreateCompatibleDC(NULL); InitializeDC(surface_); - HGDIOBJ old_bitmap = SelectObject(surface_, hbitmap); // When the memory DC is created, its display surface is exactly one - // monochrome pixel wide and one monochrome pixel high. Since we select our - // own bitmap, we must delete the previous one. - DeleteObject(old_bitmap); - // remember the hbitmap, so we can free it in our destructor - platform_extra_ = (intptr_t)hbitmap; + // monochrome pixel wide and one monochrome pixel high. Save this object + // off, we'll restore it just before deleting the memory DC. + HGDIOBJ stock_bitmap = SelectObject(surface_, hbitmap); + platform_extra_ = reinterpret_cast<intptr_t>(stock_bitmap); + + bitmap_.setConfig(SkBitmap::kARGB_8888_Config, width, height); + // PlatformBitmapPixelRef takes ownership of |hbitmap|. + bitmap_.setPixelRef( + skia::AdoptRef(new PlatformBitmapPixelRef(hbitmap, data)).get()); + bitmap_.setIsOpaque(is_opaque); + bitmap_.lockPixels(); + return true; } diff --git a/skia/ext/platform_canvas.h b/skia/ext/platform_canvas.h index 68e00072..d8fd727 100644 --- a/skia/ext/platform_canvas.h +++ b/skia/ext/platform_canvas.h @@ -10,7 +10,9 @@ #include "base/basictypes.h" #include "skia/ext/platform_device.h" #include "skia/ext/refptr.h" +#include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkCanvas.h" +#include "third_party/skia/include/core/SkPixelRef.h" namespace skia { @@ -163,6 +165,7 @@ class SK_API ScopedPlatformPaint { ScopedPlatformPaint& operator=(const ScopedPlatformPaint&); }; +// PlatformBitmap holds a PlatformSurface that can also be used as an SkBitmap. class SK_API PlatformBitmap { public: PlatformBitmap(); @@ -176,12 +179,16 @@ class SK_API PlatformBitmap { // Return the skia bitmap, which will be empty if Allocate() did not // return true. + // + // The resulting SkBitmap holds a refcount on the underlying platform surface, + // so the surface will remain allocated so long as the SkBitmap or its copies + // stay around. const SkBitmap& GetBitmap() { return bitmap_; } private: SkBitmap bitmap_; - PlatformSurface surface_; // initialized to 0 - intptr_t platform_extra_; // initialized to 0, specific to each platform + PlatformSurface surface_; // initialized to 0 + intptr_t platform_extra_; // platform specific, initialized to 0 DISALLOW_COPY_AND_ASSIGN(PlatformBitmap); }; diff --git a/skia/ext/platform_canvas_unittest.cc b/skia/ext/platform_canvas_unittest.cc index 3c8b017..572180e 100644 --- a/skia/ext/platform_canvas_unittest.cc +++ b/skia/ext/platform_canvas_unittest.cc @@ -14,11 +14,13 @@ #include <unistd.h> #endif +#include "base/memory/scoped_ptr.h" #include "skia/ext/platform_canvas.h" #include "skia/ext/platform_device.h" #include "testing/gtest/include/gtest/gtest.h" - -#include "SkColor.h" +#include "third_party/skia/include/core/SkBitmap.h" +#include "third_party/skia/include/core/SkColor.h" +#include "third_party/skia/include/core/SkPixelRef.h" namespace skia { @@ -395,4 +397,63 @@ TEST(PlatformCanvas, TranslateLayer) { #endif // #if !defined(USE_AURA) +TEST(PlatformBitmapTest, PlatformBitmap) { + const int kWidth = 400; + const int kHeight = 300; + scoped_ptr<PlatformBitmap> platform_bitmap(new PlatformBitmap); + + EXPECT_TRUE(0 == platform_bitmap->GetSurface()); + EXPECT_TRUE(platform_bitmap->GetBitmap().empty()); + EXPECT_TRUE(platform_bitmap->GetBitmap().isNull()); + + EXPECT_TRUE(platform_bitmap->Allocate(kWidth, kHeight, /*is_opaque=*/false)); + + EXPECT_TRUE(0 != platform_bitmap->GetSurface()); + EXPECT_FALSE(platform_bitmap->GetBitmap().empty()); + EXPECT_FALSE(platform_bitmap->GetBitmap().isNull()); + EXPECT_EQ(kWidth, platform_bitmap->GetBitmap().width()); + EXPECT_EQ(kHeight, platform_bitmap->GetBitmap().height()); + EXPECT_LE(platform_bitmap->GetBitmap().width()*4, + platform_bitmap->GetBitmap().rowBytes()); + EXPECT_EQ(SkBitmap::kARGB_8888_Config, // Same for all platforms. + platform_bitmap->GetBitmap().config()); + EXPECT_TRUE(platform_bitmap->GetBitmap().lockPixelsAreWritable()); + EXPECT_TRUE(platform_bitmap->GetBitmap().pixelRef()->isLocked()); + EXPECT_EQ(1, platform_bitmap->GetBitmap().pixelRef()->getRefCnt()); + + *(platform_bitmap->GetBitmap().getAddr32(10, 20)) = 0xDEED1020; + *(platform_bitmap->GetBitmap().getAddr32(20, 30)) = 0xDEED2030; + + SkBitmap sk_bitmap = platform_bitmap->GetBitmap(); + sk_bitmap.lockPixels(); + + EXPECT_EQ(2, platform_bitmap->GetBitmap().pixelRef()->getRefCnt()); + EXPECT_EQ(2, sk_bitmap.pixelRef()->getRefCnt()); + + EXPECT_EQ(0xDEED1020, *sk_bitmap.getAddr32(10, 20)); + EXPECT_EQ(0xDEED2030, *sk_bitmap.getAddr32(20, 30)); + + *(platform_bitmap->GetBitmap().getAddr32(30, 40)) = 0xDEED3040; + + // The SkBitmaps derived from a PlatformBitmap must be capable of outliving + // the PlatformBitmap. + platform_bitmap.reset(); + + EXPECT_EQ(1, sk_bitmap.pixelRef()->getRefCnt()); + + EXPECT_EQ(0xDEED1020, *sk_bitmap.getAddr32(10, 20)); + EXPECT_EQ(0xDEED2030, *sk_bitmap.getAddr32(20, 30)); + EXPECT_EQ(0xDEED3040, *sk_bitmap.getAddr32(30, 40)); + sk_bitmap.unlockPixels(); + + EXPECT_EQ(NULL, sk_bitmap.getPixels()); + + sk_bitmap.lockPixels(); + EXPECT_EQ(0xDEED1020, *sk_bitmap.getAddr32(10, 20)); + EXPECT_EQ(0xDEED2030, *sk_bitmap.getAddr32(20, 30)); + EXPECT_EQ(0xDEED3040, *sk_bitmap.getAddr32(30, 40)); + sk_bitmap.unlockPixels(); +} + + } // namespace skia diff --git a/tools/valgrind/drmemory/suppressions.txt b/tools/valgrind/drmemory/suppressions.txt index 5bfa76a..433d963 100644 --- a/tools/valgrind/drmemory/suppressions.txt +++ b/tools/valgrind/drmemory/suppressions.txt @@ -307,13 +307,6 @@ system call NtGdiDeleteObjectApp ... *!skia::BitmapPlatformDevice::BitmapPlatformDeviceData::ReleaseBitmapDC -GDI USAGE ERROR -name=http://crbug.com/109963 c -system call NtGdiDeleteObjectApp -... -skia.dll!skia::PlatformBitmap::~PlatformBitmap -*!skia::PlatformBitmap* - # GDI usage errors in 3rd-party components GDI USAGE ERROR name=http://crbug.com/119552 a diff --git a/ui/surface/accelerated_surface_win.cc b/ui/surface/accelerated_surface_win.cc index cfd0d6c..8523493 100644 --- a/ui/surface/accelerated_surface_win.cc +++ b/ui/surface/accelerated_surface_win.cc @@ -23,6 +23,7 @@ #include "base/threading/thread_restrictions.h" #include "base/time.h" #include "base/win/wrapped_window_proc.h" +#include "third_party/skia/include/core/SkBitmap.h" #include "ui/base/win/dpi.h" #include "ui/base/win/hwnd_util.h" #include "ui/base/win/shell.h" @@ -306,15 +307,13 @@ void AcceleratedPresenter::Present(HDC dc) { void AcceleratedPresenter::AsyncCopyTo( const gfx::Rect& requested_src_subrect, const gfx::Size& dst_size, - void* buf, - const base::Callback<void(bool)>& callback) { + const base::Callback<void(bool, const SkBitmap&)>& callback) { present_thread_->message_loop()->PostTask( FROM_HERE, base::Bind(&AcceleratedPresenter::DoCopyToAndAcknowledge, this, requested_src_subrect, dst_size, - buf, base::MessageLoopProxy::current(), callback)); } @@ -322,19 +321,21 @@ void AcceleratedPresenter::AsyncCopyTo( void AcceleratedPresenter::DoCopyToAndAcknowledge( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - void* buf, scoped_refptr<base::SingleThreadTaskRunner> callback_runner, - const base::Callback<void(bool)>& callback) { + const base::Callback<void(bool, const SkBitmap&)>& callback) { - bool result = DoCopyTo(src_subrect, dst_size, buf); + SkBitmap target; + bool result = DoCopyTo(src_subrect, dst_size, &target); + if (!result) + target.reset(); callback_runner->PostTask( FROM_HERE, - base::Bind(callback, result)); + base::Bind(callback, result, target)); } bool AcceleratedPresenter::DoCopyTo(const gfx::Rect& requested_src_subrect, const gfx::Size& dst_size, - void* buf) { + SkBitmap* bitmap) { TRACE_EVENT2( "gpu", "CopyTo", "width", dst_size.width(), @@ -410,19 +411,19 @@ bool AcceleratedPresenter::DoCopyTo(const gfx::Rect& requested_src_subrect, { TRACE_EVENT0("gpu", "memcpy"); - size_t bytesPerDstRow = 4 * dst_size.width(); - size_t bytesPerSrcRow = locked_rect.Pitch; - if (bytesPerDstRow == bytesPerSrcRow) { - memcpy(reinterpret_cast<int8*>(buf), - reinterpret_cast<int8*>(locked_rect.pBits), - bytesPerDstRow * dst_size.height()); - } else { - for (int i = 0; i < dst_size.height(); ++i) { - memcpy(reinterpret_cast<int8*>(buf) + bytesPerDstRow * i, - reinterpret_cast<int8*>(locked_rect.pBits) + bytesPerSrcRow * i, - bytesPerDstRow); - } + + bitmap->setConfig(SkBitmap::kARGB_8888_Config, + dst_size.width(), dst_size.height(), + locked_rect.Pitch); + if (!bitmap->allocPixels()) { + final_surface->UnlockRect(); + return false; } + bitmap->setIsOpaque(true); + + memcpy(reinterpret_cast<int8*>(bitmap->getPixels()), + reinterpret_cast<int8*>(locked_rect.pBits), + locked_rect.Pitch * dst_size.height()); } final_surface->UnlockRect(); @@ -854,9 +855,8 @@ void AcceleratedSurface::Present(HDC dc) { void AcceleratedSurface::AsyncCopyTo( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - void* buf, - const base::Callback<void(bool)>& callback) { - presenter_->AsyncCopyTo(src_subrect, dst_size, buf, callback); + const base::Callback<void(bool, const SkBitmap&)>& callback) { + presenter_->AsyncCopyTo(src_subrect, dst_size, callback); } void AcceleratedSurface::Suspend() { diff --git a/ui/surface/accelerated_surface_win.h b/ui/surface/accelerated_surface_win.h index 4dacb87..1af2482 100644 --- a/ui/surface/accelerated_surface_win.h +++ b/ui/surface/accelerated_surface_win.h @@ -64,8 +64,7 @@ class SURFACE_EXPORT AcceleratedPresenter void Present(HDC dc); void AsyncCopyTo(const gfx::Rect& src_subrect, const gfx::Size& dst_size, - void* buf, - const base::Callback<void(bool)>& callback); + const base::Callback<void(bool, const SkBitmap&)>& callback); void Invalidate(); #if defined(USE_AURA) @@ -91,12 +90,11 @@ class SURFACE_EXPORT AcceleratedPresenter void DoCopyToAndAcknowledge( const gfx::Rect& src_subrect, const gfx::Size& dst_size, - void* buf, scoped_refptr<base::SingleThreadTaskRunner> callback_runner, - const base::Callback<void(bool)>& callback); + const base::Callback<void(bool, const SkBitmap&)>& callback); bool DoCopyTo(const gfx::Rect& src_subrect, const gfx::Size& dst_size, - void* buf); + SkBitmap* bitmap); void PresentWithGDI(HDC dc); gfx::Size GetWindowSize(); @@ -174,8 +172,7 @@ class SURFACE_EXPORT AcceleratedSurface { // |4 * dst_size.width() * dst_size.height()| bytes. void AsyncCopyTo(const gfx::Rect& src_subrect, const gfx::Size& dst_size, - void* buf, - const base::Callback<void(bool)>& callback); + const base::Callback<void(bool, const SkBitmap&)>& callback); // Temporarily release resources until a new surface is asynchronously // presented. Present will not be able to represent the last surface after |