diff options
author | derat@chromium.org <derat@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-10-05 17:34:35 +0000 |
---|---|---|
committer | derat@chromium.org <derat@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-10-05 17:34:35 +0000 |
commit | b4d084526b73c9fdff84101259c12b0e88478dc2 (patch) | |
tree | 4da6ee92862eb6b60fbbec109decee9af3165757 | |
parent | d63df7f8c5f2f875632fea866376b6762ae5e481 (diff) | |
download | chromium_src-b4d084526b73c9fdff84101259c12b0e88478dc2.zip chromium_src-b4d084526b73c9fdff84101259c12b0e88478dc2.tar.gz chromium_src-b4d084526b73c9fdff84101259c12b0e88478dc2.tar.bz2 |
Revert asynchronous backing store changes.
This reverts http://codereview.chromium.org/3506007
(r61225 -- the original change) and
http://codereview.chromium.org/3616005 (r61446 -- a fix for
a double-free in Pepper caused by the original change).
Doing asynchronous copying to the backing store in the
browser requires some large changes to Pepper that I'm not
going to have time to work on anytime soon. A cache of
in-flight TransportDIBs would need to be added (similar to
what the renderer already does), and we'd also need to do
copy-on-write when a plugin wants to draw on top of a
previous frame that's currently held by the browser. It
seems safer to revert the early part of the asynchronous
copying change instead of leaving it partially checked in.
BUG=none
TEST=ran it
Review URL: http://codereview.chromium.org/3583012
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@61525 0039d316-1c4b-4281-b951-d872f2087c98
19 files changed, 61 insertions, 146 deletions
diff --git a/app/surface/transport_dib.h b/app/surface/transport_dib.h index 2f439e6..6606c2b 100644 --- a/app/surface/transport_dib.h +++ b/app/surface/transport_dib.h @@ -63,10 +63,6 @@ class TransportDIB { return other.sequence_num < sequence_num; } - bool operator==(const HandleAndSequenceNum& other) const { - return !(*this < other) && !(other < *this); - } - HANDLE handle; uint32 sequence_num; }; diff --git a/chrome/browser/renderer_host/backing_store.h b/chrome/browser/renderer_host/backing_store.h index 3977c07..53d0288 100644 --- a/chrome/browser/renderer_host/backing_store.h +++ b/chrome/browser/renderer_host/backing_store.h @@ -42,21 +42,16 @@ class BackingStore { // the backingstore to be painted from the bitmap. // // The value placed into |*painted_synchronously| indicates if the paint was - // completed synchronously and the renderer can begin generating another - // update. If the paint is being done asynchronously, the backing store will - // call DonePaintingToBackingStore() when finished. - // - // |*done_copying_bitmap| is set to true if the browser is done using - // the TransportDIB and to false otherwise. In the false case, the backing - // store will manage notifying the RenderWidgetHost that it's done with it via - // DoneCopyingBitmapToBackingStore(). + // completed synchronously and the TransportDIB can be freed. False means that + // the backing store may still be using the transport DIB and it will manage + // notifying the RenderWidgetHost that it's done with it via + // DonePaintingToBackingStore(). virtual void PaintToBackingStore( RenderProcessHost* process, TransportDIB::Id bitmap, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, - bool* painted_synchronously, - bool* done_copying_bitmap) = 0; + bool* painted_synchronously) = 0; // Extracts the gives subset of the backing store and copies it to the given // PlatformCanvas. The PlatformCanvas should not be initialized. This function diff --git a/chrome/browser/renderer_host/backing_store_mac.h b/chrome/browser/renderer_host/backing_store_mac.h index 0c1df14..0c1e4a7 100644 --- a/chrome/browser/renderer_host/backing_store_mac.h +++ b/chrome/browser/renderer_host/backing_store_mac.h @@ -29,8 +29,7 @@ class BackingStoreMac : public BackingStore { TransportDIB::Id bitmap, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, - bool* painted_synchronously, - bool* done_copying_bitmap); + bool* painted_synchronously); virtual bool CopyFromBackingStore(const gfx::Rect& rect, skia::PlatformCanvas* output); virtual void ScrollBackingStore(int dx, int dy, diff --git a/chrome/browser/renderer_host/backing_store_mac.mm b/chrome/browser/renderer_host/backing_store_mac.mm index d7156ea..f793395 100644 --- a/chrome/browser/renderer_host/backing_store_mac.mm +++ b/chrome/browser/renderer_host/backing_store_mac.mm @@ -56,12 +56,10 @@ void BackingStoreMac::PaintToBackingStore( TransportDIB::Id bitmap, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, - bool* painted_synchronously, - bool* done_copying_bitmap) { + bool* painted_synchronously) { // Our paints are always synchronous and the caller can free the TransportDIB, // even on failure. *painted_synchronously = true; - *done_copying_bitmap = true; DCHECK_NE(static_cast<bool>(cg_layer()), static_cast<bool>(cg_bitmap())); diff --git a/chrome/browser/renderer_host/backing_store_manager.cc b/chrome/browser/renderer_host/backing_store_manager.cc index 9b7cfa2..4325f31 100644 --- a/chrome/browser/renderer_host/backing_store_manager.cc +++ b/chrome/browser/renderer_host/backing_store_manager.cc @@ -197,11 +197,9 @@ void BackingStoreManager::PrepareBackingStore( const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, bool* needs_full_paint, - bool* painted_synchronously, - bool* done_copying_bitmap) { + bool* painted_synchronously) { // Default to declaring we're done using the transport DIB so it can be freed. *painted_synchronously = true; - *done_copying_bitmap = true; BackingStore* backing_store = GetBackingStore(host, backing_store_size); if (!backing_store) { @@ -219,12 +217,9 @@ void BackingStoreManager::PrepareBackingStore( backing_store = CreateBackingStore(host, backing_store_size); } - backing_store->PaintToBackingStore(host->process(), - bitmap, - bitmap_rect, - copy_rects, - painted_synchronously, - done_copying_bitmap); + backing_store->PaintToBackingStore(host->process(), bitmap, + bitmap_rect, copy_rects, + painted_synchronously); } // static diff --git a/chrome/browser/renderer_host/backing_store_manager.h b/chrome/browser/renderer_host/backing_store_manager.h index 38f6574..1ab78cc 100644 --- a/chrome/browser/renderer_host/backing_store_manager.h +++ b/chrome/browser/renderer_host/backing_store_manager.h @@ -44,10 +44,10 @@ class BackingStoreManager { // Set if we need to send out a request to paint the view // to the renderer. // painted_synchronously - // Will be set by the function if the request was processed synchronously. - // done_copying_bitmap - // False means that the backing store is still using the TransportDIB and it - // can't be freed (it will be the backing store's job to free it later). + // Will be set by the function if the request was processed synchronously, + // and the bitmap is done being used. False means that the backing store + // will paint the bitmap at a later time and that the TransportDIB can't be + // freed (it will be the backing store's job to free it later). static void PrepareBackingStore( RenderWidgetHost* host, const gfx::Size& backing_store_size, @@ -55,8 +55,7 @@ class BackingStoreManager { const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, bool* needs_full_paint, - bool* painted_synchronously, - bool* done_copying_bitmap); + bool* painted_synchronously); // Returns a matching backing store for the host. // Returns NULL if we fail to find one. diff --git a/chrome/browser/renderer_host/backing_store_proxy.cc b/chrome/browser/renderer_host/backing_store_proxy.cc index 40c5636..ba3538b 100644 --- a/chrome/browser/renderer_host/backing_store_proxy.cc +++ b/chrome/browser/renderer_host/backing_store_proxy.cc @@ -23,8 +23,7 @@ BackingStoreProxy::BackingStoreProxy(RenderWidgetHost* widget, : BackingStore(widget, size), process_shim_(process_shim), routing_id_(routing_id), - waiting_for_paint_ack_(false), - current_bitmap_(TransportDIB::Id()) { + waiting_for_paint_ack_(false) { process_shim_->AddRoute(routing_id_, this); } @@ -37,8 +36,7 @@ void BackingStoreProxy::PaintToBackingStore( TransportDIB::Id bitmap, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, - bool* painted_synchronously, - bool* done_copying_bitmap) { + bool* painted_synchronously) { DCHECK(!waiting_for_paint_ack_); base::ProcessId process_id; @@ -51,17 +49,12 @@ void BackingStoreProxy::PaintToBackingStore( if (process_shim_->Send(new GpuMsg_PaintToBackingStore( routing_id_, process_id, bitmap, bitmap_rect, copy_rects))) { // Message sent successfully, so the caller can not destroy the - // TransportDIB. DoneCopyingBitmapToBackingStore will release it later, and - // DonePaintingToBackingStore will let the renderer know that it can send us - // the next update. + // TransportDIB. OnDonePaintingToBackingStore will free it later. *painted_synchronously = false; - *done_copying_bitmap = false; waiting_for_paint_ack_ = true; - current_bitmap_ = bitmap; } else { // On error, we're done with the TransportDIB and the caller can free it. *painted_synchronously = true; - *done_copying_bitmap = true; } } @@ -103,8 +96,6 @@ void BackingStoreProxy::OnChannelError() { void BackingStoreProxy::OnPaintToBackingStoreACK() { DCHECK(waiting_for_paint_ack_); - render_widget_host()->DoneCopyingBitmapToBackingStore(current_bitmap_); render_widget_host()->DonePaintingToBackingStore(); waiting_for_paint_ack_ = false; - current_bitmap_ = TransportDIB::Id(); } diff --git a/chrome/browser/renderer_host/backing_store_proxy.h b/chrome/browser/renderer_host/backing_store_proxy.h index 0976b58..610f7ef 100644 --- a/chrome/browser/renderer_host/backing_store_proxy.h +++ b/chrome/browser/renderer_host/backing_store_proxy.h @@ -12,7 +12,6 @@ class GpuProcessHostUIShim; -// This class proxies renderer updates to the GPU process. class BackingStoreProxy : public BackingStore, public IPC::Channel::Listener { public: @@ -25,8 +24,7 @@ class BackingStoreProxy : public BackingStore, TransportDIB::Id bitmap, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, - bool* painted_synchronously, - bool* done_copying_bitmap); + bool* painted_synchronously); virtual bool CopyFromBackingStore(const gfx::Rect& rect, skia::PlatformCanvas* output); virtual void ScrollBackingStore(int dx, int dy, @@ -47,12 +45,9 @@ class BackingStoreProxy : public BackingStore, // Set to true when we're waiting for the GPU process to do a paint and send // back a "done" message. In this case, the renderer will be waiting for our - // message that we're done using the bitmap. + // message that we're done using the backing store. bool waiting_for_paint_ack_; - // Bitmap currently being used by the GPU process. - TransportDIB::Id current_bitmap_; - DISALLOW_COPY_AND_ASSIGN(BackingStoreProxy); }; diff --git a/chrome/browser/renderer_host/backing_store_win.cc b/chrome/browser/renderer_host/backing_store_win.cc index b13d652..94d9cbf 100644 --- a/chrome/browser/renderer_host/backing_store_win.cc +++ b/chrome/browser/renderer_host/backing_store_win.cc @@ -69,8 +69,7 @@ void CallStretchDIBits(HDC hdc, int dest_x, int dest_y, int dest_w, int dest_h, } // namespace -BackingStoreWin::BackingStoreWin(RenderWidgetHost* widget, - const gfx::Size& size) +BackingStoreWin::BackingStoreWin(RenderWidgetHost* widget, const gfx::Size& size) : BackingStore(widget, size), backing_store_dib_(NULL), original_bitmap_(NULL) { @@ -118,12 +117,10 @@ void BackingStoreWin::PaintToBackingStore( TransportDIB::Id bitmap, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, - bool* painted_synchronously, - bool* done_copying_bitmap) { + bool* painted_synchronously) { // Our paints are always synchronous and the TransportDIB can be freed when // we're done (even on error). *painted_synchronously = true; - *done_copying_bitmap = true; if (!backing_store_dib_) { backing_store_dib_ = CreateDIB(hdc_, size().width(), diff --git a/chrome/browser/renderer_host/backing_store_win.h b/chrome/browser/renderer_host/backing_store_win.h index 4ea1480..221e0aa 100644 --- a/chrome/browser/renderer_host/backing_store_win.h +++ b/chrome/browser/renderer_host/backing_store_win.h @@ -27,8 +27,7 @@ class BackingStoreWin : public BackingStore { TransportDIB::Id bitmap, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, - bool* painted_synchronously, - bool* done_copying_bitmap); + bool* painted_synchronously); virtual bool CopyFromBackingStore(const gfx::Rect& rect, skia::PlatformCanvas* output); virtual void ScrollBackingStore(int dx, int dy, diff --git a/chrome/browser/renderer_host/backing_store_x.cc b/chrome/browser/renderer_host/backing_store_x.cc index a7750ae4..f2704c8 100644 --- a/chrome/browser/renderer_host/backing_store_x.cc +++ b/chrome/browser/renderer_host/backing_store_x.cc @@ -161,12 +161,10 @@ void BackingStoreX::PaintToBackingStore( TransportDIB::Id bitmap, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, - bool* painted_synchronously, - bool* done_copying_bitmap) { + bool* painted_synchronously) { // Our paints are always synchronous and the caller can free the TransportDIB // when we're done, even on error. *painted_synchronously = true; - *done_copying_bitmap = true; if (!display_) return; diff --git a/chrome/browser/renderer_host/backing_store_x.h b/chrome/browser/renderer_host/backing_store_x.h index 816d520..283c19f 100644 --- a/chrome/browser/renderer_host/backing_store_x.h +++ b/chrome/browser/renderer_host/backing_store_x.h @@ -54,8 +54,7 @@ class BackingStoreX : public BackingStore { TransportDIB::Id bitmap, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, - bool* painted_synchronously, - bool* done_copying_bitmap); + bool* painted_synchronously); virtual bool CopyFromBackingStore(const gfx::Rect& rect, skia::PlatformCanvas* output); virtual void ScrollBackingStore(int dx, int dy, diff --git a/chrome/browser/renderer_host/render_widget_host.cc b/chrome/browser/renderer_host/render_widget_host.cc index f2120cf..fe50d69 100644 --- a/chrome/browser/renderer_host/render_widget_host.cc +++ b/chrome/browser/renderer_host/render_widget_host.cc @@ -370,11 +370,6 @@ BackingStore* RenderWidgetHost::AllocBackingStore(const gfx::Size& size) { return view_->AllocBackingStore(size); } -void RenderWidgetHost::DoneCopyingBitmapToBackingStore( - TransportDIB::Id bitmap) { - Send(new ViewMsg_DoneUsingBitmap(routing_id(), bitmap)); -} - void RenderWidgetHost::DonePaintingToBackingStore() { Send(new ViewMsg_UpdateRect_ACK(routing_id())); } @@ -797,7 +792,6 @@ void RenderWidgetHost::OnMsgUpdateRect( DCHECK(!params.view_size.IsEmpty()); bool painted_synchronously = true; // Default to sending a paint ACK below. - bool done_copying_bitmap = true; if (!is_gpu_rendering_active_) { const size_t size = params.bitmap_rect.height() * params.bitmap_rect.width() * 4; @@ -820,25 +814,20 @@ void RenderWidgetHost::OnMsgUpdateRect( // Paint the backing store. This will update it with the // renderer-supplied bits. The view will read out of the backing store // later to actually draw to the screen. - PaintBackingStoreRect(params.bitmap, - params.bitmap_rect, - params.copy_rects, - params.view_size, - &painted_synchronously, - &done_copying_bitmap); + PaintBackingStoreRect(params.bitmap, params.bitmap_rect, + params.copy_rects, params.view_size, + &painted_synchronously); } } } - // Tell the renderer that the bitmap can be re-used, so the bitmap may be - // invalid after this call. If the backing store is still using the bitmap, it - // will manage issuing this IPC. - if (done_copying_bitmap) - DoneCopyingBitmapToBackingStore(params.bitmap); - - // ACK early so we can prefetch the next UpdateRect if there is a next one. + // ACK early so we can prefetch the next PaintRect if there is a next one. + // This must be done AFTER we're done painting with the bitmap supplied by the + // renderer. This ACK is a signal to the renderer that the backing store can + // be re-used, so the bitmap may be invalid after this call. If the backing + // store is painting asynchronously, it will manage issuing this IPC. if (painted_synchronously) - DonePaintingToBackingStore(); + Send(new ViewMsg_UpdateRect_ACK(routing_id_)); // We don't need to update the view if the view is hidden. We must do this // early return after the ACK is sent, however, or the renderer will not send @@ -1094,12 +1083,10 @@ void RenderWidgetHost::PaintBackingStoreRect( const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, const gfx::Size& view_size, - bool* painted_synchronously, - bool* done_copying_bitmap) { + bool* painted_synchronously) { // On failure, we need to be sure our caller knows we're done with the // backing store. *painted_synchronously = true; - *done_copying_bitmap = true; // The view may be destroyed already. if (!view_) @@ -1114,14 +1101,9 @@ void RenderWidgetHost::PaintBackingStoreRect( } bool needs_full_paint = false; - BackingStoreManager::PrepareBackingStore(this, - view_size, - bitmap, - bitmap_rect, - copy_rects, - &needs_full_paint, - painted_synchronously, - done_copying_bitmap); + BackingStoreManager::PrepareBackingStore(this, view_size, bitmap, bitmap_rect, + copy_rects, &needs_full_paint, + painted_synchronously); if (needs_full_paint) { repaint_start_time_ = TimeTicks::Now(); repaint_ack_pending_ = true; diff --git a/chrome/browser/renderer_host/render_widget_host.h b/chrome/browser/renderer_host/render_widget_host.h index b8acd4e..f4e0b96 100644 --- a/chrome/browser/renderer_host/render_widget_host.h +++ b/chrome/browser/renderer_host/render_widget_host.h @@ -238,14 +238,9 @@ class RenderWidgetHost : public IPC::Channel::Listener, // (for example, if we don't currently have a RenderWidgetHostView.) BackingStore* AllocBackingStore(const gfx::Size& size); - // When a backing store is copying a TransportDIB asynchronously, it calls - // this function when it is done using it. We then send a message to the - // renderer letting it know that the bitmap can be reused. - void DoneCopyingBitmapToBackingStore(TransportDIB::Id bitmap); - - // When a backing store does asynchronous painting, it calls this function - // when it is done painting. We then send a message to the renderer to send - // another update. + // When a backing store does asynchronous painting, it will call this function + // when it is done with the DIB. We will then forward a message to the + // renderer to send another paint. void DonePaintingToBackingStore(); // GPU accelerated version of GetBackingStore function. This will @@ -531,8 +526,7 @@ class RenderWidgetHost : public IPC::Channel::Listener, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, const gfx::Size& view_size, - bool* painted_synchronously, - bool* done_copying_bitmap); + bool* painted_synchronously); // Scrolls the given |clip_rect| in the backing by the given dx/dy amount. The // |dib| and its corresponding location |bitmap_rect| in the backing store diff --git a/chrome/browser/renderer_host/test/test_backing_store.cc b/chrome/browser/renderer_host/test/test_backing_store.cc index 8e2a9b8..d90c7fd 100644 --- a/chrome/browser/renderer_host/test/test_backing_store.cc +++ b/chrome/browser/renderer_host/test/test_backing_store.cc @@ -17,8 +17,7 @@ void TestBackingStore::PaintToBackingStore( TransportDIB::Id bitmap, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, - bool* painted_synchronously, - bool* done_copying_bitmap) { + bool* painted_synchronously) { } bool TestBackingStore::CopyFromBackingStore(const gfx::Rect& rect, diff --git a/chrome/browser/renderer_host/test/test_backing_store.h b/chrome/browser/renderer_host/test/test_backing_store.h index ee5ffce..f9db76e 100644 --- a/chrome/browser/renderer_host/test/test_backing_store.h +++ b/chrome/browser/renderer_host/test/test_backing_store.h @@ -19,8 +19,7 @@ class TestBackingStore : public BackingStore { TransportDIB::Id bitmap, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, - bool* painted_synchronously, - bool* done_copying_bitmap); + bool* painted_synchronously); virtual bool CopyFromBackingStore(const gfx::Rect& rect, skia::PlatformCanvas* output); virtual void ScrollBackingStore(int dx, int dy, diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h index 326202a..ae4ebcb 100644 --- a/chrome/common/render_messages_internal.h +++ b/chrome/common/render_messages_internal.h @@ -180,11 +180,6 @@ IPC_BEGIN_MESSAGES(View) gfx::Size /* page_size */, gfx::Size /* desired_size */) - // Sent when the backing store is finished using a TransportDIB received from - // the renderer via a ViewHostMsg_UpdateRect message. - IPC_MESSAGE_ROUTED1(ViewMsg_DoneUsingBitmap, - TransportDIB::Id /* bitmap */) - // Tells the render view that a ViewHostMsg_UpdateRect message was processed. // This signals the render view that it can send another UpdateRect message. IPC_MESSAGE_ROUTED0(ViewMsg_UpdateRect_ACK) diff --git a/chrome/renderer/render_widget.cc b/chrome/renderer/render_widget.cc index 90447c4..f5a7ebe 100644 --- a/chrome/renderer/render_widget.cc +++ b/chrome/renderer/render_widget.cc @@ -4,8 +4,6 @@ #include "chrome/renderer/render_widget.h" -#include <utility> - #include "app/surface/transport_dib.h" #include "base/command_line.h" #include "base/logging.h" @@ -63,6 +61,7 @@ RenderWidget::RenderWidget(RenderThreadBase* render_thread, opener_id_(MSG_ROUTING_NONE), render_thread_(render_thread), host_window_(0), + current_paint_buf_(NULL), next_paint_flags_(0), update_reply_pending_(false), did_show_(false), @@ -83,9 +82,9 @@ RenderWidget::RenderWidget(RenderThreadBase* render_thread, RenderWidget::~RenderWidget() { DCHECK(!webwidget_) << "Leaking our WebWidget!"; - for (TransportDIBMap::iterator it = current_paint_bufs_.begin(); - it != current_paint_bufs_.end(); ++it) { - RenderProcess::current()->ReleaseTransportDIB(it->second); + if (current_paint_buf_) { + RenderProcess::current()->ReleaseTransportDIB(current_paint_buf_); + current_paint_buf_ = NULL; } RenderProcess::current()->ReleaseProcess(); } @@ -170,7 +169,6 @@ IPC_DEFINE_MESSAGE_MAP(RenderWidget) IPC_MESSAGE_HANDLER(ViewMsg_Resize, OnResize) IPC_MESSAGE_HANDLER(ViewMsg_WasHidden, OnWasHidden) IPC_MESSAGE_HANDLER(ViewMsg_WasRestored, OnWasRestored) - IPC_MESSAGE_HANDLER(ViewMsg_DoneUsingBitmap, OnDoneUsingBitmap) IPC_MESSAGE_HANDLER(ViewMsg_UpdateRect_ACK, OnUpdateRectAck) IPC_MESSAGE_HANDLER(ViewMsg_CreateVideo_ACK, OnCreateVideoAck) IPC_MESSAGE_HANDLER(ViewMsg_UpdateVideo_ACK, OnUpdateVideoAck) @@ -301,26 +299,17 @@ void RenderWidget::OnRequestMoveAck() { pending_window_rect_count_--; } -void RenderWidget::OnDoneUsingBitmap(TransportDIB::Id id) { - // If we sent an UpdateRect message with a zero-sized bitmap, then we should - // get an empty bitmap in response. - // TODO(derat): Don't send the "done using bitmap" message at all in this - // case. - if (id == TransportDIB::Id()) - return; - - TransportDIBMap::iterator it = current_paint_bufs_.find(id); - if (it != current_paint_bufs_.end()) { - RenderProcess::current()->ReleaseTransportDIB(it->second); - current_paint_bufs_.erase(it); - return; - } -} - void RenderWidget::OnUpdateRectAck() { DCHECK(update_reply_pending()); update_reply_pending_ = false; + // If we sent an UpdateRect message with a zero-sized bitmap, then we should + // have no current paint buffer. + if (current_paint_buf_) { + RenderProcess::current()->ReleaseTransportDIB(current_paint_buf_); + current_paint_buf_ = NULL; + } + // Notify subclasses. DidFlushPaint(); @@ -532,12 +521,12 @@ void RenderWidget::DoDeferredUpdate() { } else if (!is_gpu_rendering_active_) { // Compute a buffer for painting and cache it. scoped_ptr<skia::PlatformCanvas> canvas( - RenderProcess::current()->GetDrawingCanvas(&dib, bounds)); + RenderProcess::current()->GetDrawingCanvas(¤t_paint_buf_, + bounds)); if (!canvas.get()) { NOTREACHED(); return; } - current_paint_bufs_.insert(std::make_pair(dib->id(), dib)); // We may get back a smaller canvas than we asked for. // TODO(darin): This seems like it could cause painting problems! @@ -563,7 +552,7 @@ void RenderWidget::DoDeferredUpdate() { for (size_t i = 0; i < copy_rects.size(); ++i) PaintRect(copy_rects[i], bounds.origin(), canvas.get()); - dib_id = dib->id(); + dib_id = current_paint_buf_->id(); } else { // Accelerated compositing path // Begin painting. bool finish = next_paint_is_resize_ack(); diff --git a/chrome/renderer/render_widget.h b/chrome/renderer/render_widget.h index 8aede6c..47bdf38 100644 --- a/chrome/renderer/render_widget.h +++ b/chrome/renderer/render_widget.h @@ -6,7 +6,6 @@ #define CHROME_RENDERER_RENDER_WIDGET_H_ #pragma once -#include <map> #include <vector> #include "app/surface/transport_dib.h" @@ -130,8 +129,6 @@ class RenderWidget : public IPC::Channel::Listener, // without ref-counting is an error. friend class base::RefCounted<RenderWidget>; - typedef std::map<TransportDIB::Id, TransportDIB*> TransportDIBMap; - RenderWidget(RenderThreadBase* render_thread, WebKit::WebPopupType popup_type); virtual ~RenderWidget(); @@ -174,7 +171,6 @@ class RenderWidget : public IPC::Channel::Listener, const gfx::Rect& resizer_rect); virtual void OnWasHidden(); virtual void OnWasRestored(bool needs_repainting); - void OnDoneUsingBitmap(TransportDIB::Id id); void OnUpdateRectAck(); void OnCreateVideoAck(int32 video_id); void OnUpdateVideoAck(int32 video_id); @@ -292,8 +288,8 @@ class RenderWidget : public IPC::Channel::Listener, // The size of the RenderWidget. gfx::Size size_; - // TransportDIBs currently being used to transfer images to the browser. - TransportDIBMap current_paint_bufs_; + // The TransportDIB that is being used to transfer an image to the browser. + TransportDIB* current_paint_buf_; PaintAggregator paint_aggregator_; |