diff options
author | kkania@chromium.org <kkania@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-10-20 18:00:36 +0000 |
---|---|---|
committer | kkania@chromium.org <kkania@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-10-20 18:00:36 +0000 |
commit | ae849ea693102dd66d5f01d9ad6799e0bde070d0 (patch) | |
tree | 2fdafb8dd84f96c9e61a6b5831860e4ba8406aba | |
parent | d62b5fc77d374524bed3752950f268851b8585c6 (diff) | |
download | chromium_src-ae849ea693102dd66d5f01d9ad6799e0bde070d0.zip chromium_src-ae849ea693102dd66d5f01d9ad6799e0bde070d0.tar.gz chromium_src-ae849ea693102dd66d5f01d9ad6799e0bde070d0.tar.bz2 |
On Windows, create a new TransportDIB::Handle struct which includes the file
mapping HANDLE and the source process ID. Duplicating the handle for the
remote process is done in TransportDIB::Map, instead of in various #ifdefs
scattered across the code. Also on windows, remove the struct for the
TransportDIB::Id which contained both the sequence number and the HANDLE and
replace it with just the sequence number.
Fix ThumbnailGenerator by mapping the TransportDIB on Windows and adding
a method to duplicate the file mapping handle before sending across the
channel.
Also, add a ScopedHandle and fix some handle leaks.
BUG=none
TEST=none
Review URL: http://codereview.chromium.org/3834003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@63232 0039d316-1c4b-4281-b951-d872f2087c98
48 files changed, 538 insertions, 323 deletions
diff --git a/app/surface/transport_dib.h b/app/surface/transport_dib.h index 6606c2b..37a3912 100644 --- a/app/surface/transport_dib.h +++ b/app/surface/transport_dib.h @@ -7,6 +7,7 @@ #pragma once #include "base/basictypes.h" +#include "base/process.h" #if defined(OS_WIN) || defined(OS_MACOSX) #include "base/shared_memory.h" @@ -31,13 +32,55 @@ class TransportDIB { public: ~TransportDIB(); - // Two typedefs are defined. A Handle is the type which can be sent over - // the wire so that the remote side can map the transport DIB. The Id typedef - // is sufficient to identify the transport DIB when you know that the remote - // side already may have it mapped. + // Two typedefs are defined. A |Handle| can be sent over the wire so that the + // remote side can map the |TransportDIB|. These handles may be reused from + // previous DIBs. An |Id| is unique and never reused, but it is not sufficient + // to map the DIB. #if defined(OS_WIN) - typedef HANDLE Handle; - // On Windows, the Id type includes a sequence number (epoch) to solve an ABA + // On Windows, a |Handle| is a combination of the section (i.e., file mapping) + // handle and the ID of the corresponding process. When the DIB is mapped in + // a remote process, the section handle is duplicated for use in that process. + // However, if the remote process does not have permission to duplicate the + // handle, the first process must duplicate the handle before sending it. + // E.g., this is necessary if the DIB is created in the browser and will be + // mapped in the sandboxed renderer. + class TransferrableSectionHandle { + public: + TransferrableSectionHandle() + : section_(NULL), owner_id_(NULL), should_dup_on_map_(false) { + } + + TransferrableSectionHandle(HANDLE section, base::ProcessId owner_id, + bool should_dup_on_map) + : section_(section), + owner_id_(owner_id), + should_dup_on_map_(should_dup_on_map) { + } + + // Duplicates the handle for use in the given process. + TransferrableSectionHandle DupForProcess( + base::ProcessHandle new_owner) const; + + // Closes this handle. This should be called if this handle was duplicated + // and is not owned by a TransportDIB. + void Close() const; + + // Returns true if this handle refers to an actual file mapping. + bool is_valid() const { return section_ != NULL && owner_id_ != NULL; } + + HANDLE section() const { return section_; } + base::ProcessId owner_id() const { return owner_id_; } + bool should_dup_on_map() const { return should_dup_on_map_; } + + private: + HANDLE section_; + base::ProcessId owner_id_; + // Whether the handle should be duplicated when the DIB is mapped. + bool should_dup_on_map_; + }; + typedef TransferrableSectionHandle Handle; + + // On Windows, the Id type is a sequence number (epoch) to solve an ABA // issue: // 1) Process A creates a transport DIB with HANDLE=1 and sends to B. // 2) Process B maps the transport DIB and caches 1 -> DIB. @@ -45,38 +88,17 @@ class TransportDIB { // is also assigned HANDLE=1. // 4) Process A sends the Handle to B, but B incorrectly believes that it // already has it cached. - struct HandleAndSequenceNum { - HandleAndSequenceNum() - : handle(NULL), - sequence_num(0) { - } - - HandleAndSequenceNum(HANDLE h, uint32 seq_num) - : handle(h), - sequence_num(seq_num) { - } - - bool operator< (const HandleAndSequenceNum& other) const { - // Use the lexicographic order on the tuple <handle, sequence_num>. - if (other.handle != handle) - return other.handle < handle; - return other.sequence_num < sequence_num; - } - - HANDLE handle; - uint32 sequence_num; - }; - typedef HandleAndSequenceNum Id; + typedef uint32 Id; // Returns a default, invalid handle, that is meant to indicate a missing // Transport DIB. - static Handle DefaultHandleValue() { return NULL; } + static Handle DefaultHandleValue() { return Handle(); } // Returns a value that is ONLY USEFUL FOR TESTS WHERE IT WON'T BE // ACTUALLY USED AS A REAL HANDLE. static Handle GetFakeHandleForTest() { static int fake_handle = 10; - return reinterpret_cast<Handle>(fake_handle++); + return Handle(reinterpret_cast<HANDLE>(fake_handle++), 1, false); } #elif defined(OS_MACOSX) typedef base::SharedMemoryHandle Handle; @@ -109,7 +131,41 @@ class TransportDIB { } #endif - // Create a new TransportDIB, returning NULL on failure. + // When passing a TransportDIB::Handle across processes, you must always close + // the handle, even if you return early, or the handle will be leaked. Typical + // usage will be: + // + // MyIPCHandler(TransportDIB::Handle dib_handle) { + // TransportDIB::ScopedHandle handle_scoper(dib_handle); + // ... do some stuff, possible returning early ... + // + // TransportDIB* dib = TransportDIB::Map(handle_scoper.release()); + // // The handle lifetime is now managed by the TransportDIB. + class ScopedHandle { + public: + ScopedHandle() : handle_(DefaultHandleValue()) {} + explicit ScopedHandle(Handle handle) : handle_(handle) {} + + ~ScopedHandle() { + Close(); + } + + Handle release() { + Handle temp = handle_; + handle_ = DefaultHandleValue(); + return temp; + } + + operator Handle() { return handle_; } + + private: + void Close(); + + Handle handle_; + DISALLOW_COPY_AND_ASSIGN(ScopedHandle); + }; + + // Create a new |TransportDIB|, returning NULL on failure. // // The size is the minimum size in bytes of the memory backing the transport // DIB (we may actually allocate more than that to give us better reuse when @@ -118,12 +174,18 @@ class TransportDIB { // The sequence number is used to uniquely identify the transport DIB. It // should be unique for all transport DIBs ever created in the same // renderer. + // + // On Linux, this will also map the DIB into the current process. static TransportDIB* Create(size_t size, uint32 sequence_num); - // Map the referenced transport DIB. The caller owns the returned object. + // Map the referenced transport DIB. The caller owns the returned object. // Returns NULL on failure. static TransportDIB* Map(Handle transport_dib); + // Create a new |TransportDIB| with a handle to the shared memory. This + // always returns a valid pointer. The DIB is not mapped. + static TransportDIB* CreateWithHandle(Handle handle); + // Returns true if the handle is valid. static bool is_valid(Handle dib); @@ -131,11 +193,31 @@ class TransportDIB { // pointer will be owned by the caller. The bitmap will be of the given size, // which should fit inside this memory. // + // On POSIX, this |TransportDIB| will be mapped if not already. On Windows, + // this |TransportDIB| will NOT be mapped and should not be mapped prior, + // because PlatformCanvas will map the file internally. + // // Will return NULL on allocation failure. This could be because the image // is too large to map into the current process' address space. skia::PlatformCanvas* GetPlatformCanvas(int w, int h); - // Return a pointer to the shared memory + // Map the DIB into the current process if it is not already. This is used to + // map a DIB that has already been created. Returns true if the DIB is mapped. + bool Map(); + + // Return a handle for use in a specific process. On POSIX, this simply + // returns the handle as in the |handle| accessor below. On Windows, this + // returns a duplicate handle for use in the given process. This should be + // used instead of the |handle| accessor only if the process that will map + // this DIB does not have permission to duplicate the handle from the + // first process. + // + // Note: On Windows, if the duplicated handle is not closed by the other side + // (or this process fails to transmit the handle), the shared memory will be + // leaked. + Handle GetHandleForProcess(base::ProcessHandle process_handle) const; + + // Return a pointer to the shared memory. void* memory() const; // Return the maximum size of the shared memory. This is not the amount of diff --git a/app/surface/transport_dib_linux.cc b/app/surface/transport_dib_linux.cc index 26cad3f..ac8d71b 100644 --- a/app/surface/transport_dib_linux.cc +++ b/app/surface/transport_dib_linux.cc @@ -17,6 +17,9 @@ // The shmat system call uses this as it's invalid return address static void *const kInvalidAddress = (void*) -1; +void TransportDIB::ScopedHandle::Close() { +} + TransportDIB::TransportDIB() : key_(-1), address_(kInvalidAddress), @@ -65,34 +68,57 @@ TransportDIB* TransportDIB::Create(size_t size, uint32 sequence_num) { return dib; } -TransportDIB* TransportDIB::Map(Handle shmkey) { - struct shmid_ds shmst; - if (shmctl(shmkey, IPC_STAT, &shmst) == -1) - return NULL; - - void* address = shmat(shmkey, NULL /* desired address */, 0 /* flags */); - if (address == kInvalidAddress) +// static +TransportDIB* TransportDIB::Map(Handle handle) { + scoped_ptr<TransportDIB> dib(CreateWithHandle(handle)); + if (!dib->Map()) return NULL; + return dib.release(); +} +// static +TransportDIB* TransportDIB::CreateWithHandle(Handle shmkey) { TransportDIB* dib = new TransportDIB; - - dib->address_ = address; - dib->size_ = shmst.shm_segsz; dib->key_ = shmkey; return dib; } +// static bool TransportDIB::is_valid(Handle dib) { return dib >= 0; } skia::PlatformCanvas* TransportDIB::GetPlatformCanvas(int w, int h) { + if (address_ == kInvalidAddress && !Map()) + return NULL; scoped_ptr<skia::PlatformCanvas> canvas(new skia::PlatformCanvas); if (!canvas->initialize(w, h, true, reinterpret_cast<uint8_t*>(memory()))) return NULL; return canvas.release(); } +bool TransportDIB::Map() { + if (address_ != kInvalidAddress) + return true; + + struct shmid_ds shmst; + if (shmctl(key_, IPC_STAT, &shmst) == -1) + return false; + + void* address = shmat(key_, NULL /* desired address */, 0 /* flags */); + if (address == kInvalidAddress) + return false; + + address_ = address; + size_ = shmst.shm_segsz; + return true; +} + +TransportDIB::Handle TransportDIB::GetHandleForProcess( + base::ProcessHandle process_handle) const { + return handle(); +} + void* TransportDIB::memory() const { DCHECK_NE(address_, kInvalidAddress); return address_; diff --git a/app/surface/transport_dib_mac.cc b/app/surface/transport_dib_mac.cc index a3eb0bb..68ccfca 100644 --- a/app/surface/transport_dib_mac.cc +++ b/app/surface/transport_dib_mac.cc @@ -13,6 +13,11 @@ #include "base/shared_memory.h" #include "skia/ext/platform_canvas.h" +void TransportDIB::ScopedHandle::Close() { + if (is_valid(handle_)) + base::SharedMemory::CloseHandle(handle_); +} + TransportDIB::TransportDIB() : size_(0) { } @@ -34,46 +39,56 @@ TransportDIB* TransportDIB::Create(size_t size, uint32 sequence_num) { return NULL; } - if (!dib->shared_memory_.Map(size)) { - delete dib; - return NULL; - } - dib->size_ = size; return dib; } // static -TransportDIB* TransportDIB::Map(TransportDIB::Handle handle) { - if (!is_valid(handle)) +TransportDIB* TransportDIB::Map(Handle handle) { + scoped_ptr<TransportDIB> dib(CreateWithHandle(handle)); + if (!dib->Map()) return NULL; + return dib.release(); +} - TransportDIB* dib = new TransportDIB(handle); - struct stat st; - if ((fstat(handle.fd, &st) != 0) || - (!dib->shared_memory_.Map(st.st_size))) { - delete dib; - if (HANDLE_EINTR(close(handle.fd)) < 0) - PLOG(ERROR) << "close"; - return NULL; - } - - dib->size_ = st.st_size; - - return dib; +// static +TransportDIB* TransportDIB::CreateWithHandle(Handle handle) { + return new TransportDIB(handle); } +// static bool TransportDIB::is_valid(Handle dib) { return dib.fd >= 0; } skia::PlatformCanvas* TransportDIB::GetPlatformCanvas(int w, int h) { + if (!memory() && !Map()) + return NULL; scoped_ptr<skia::PlatformCanvas> canvas(new skia::PlatformCanvas); if (!canvas->initialize(w, h, true, reinterpret_cast<uint8_t*>(memory()))) return NULL; return canvas.release(); } +bool TransportDIB::Map() { + if (memory()) + return true; + + struct stat st; + if ((fstat(shared_memory_.handle().fd, &st) != 0) || + (!shared_memory_.Map(st.st_size))) { + return false; + } + + size_ = st.st_size; + return true; +} + +TransportDIB::Handle TransportDIB::GetHandleForProcess( + base::ProcessHandle process_handle) const { + return handle(); +} + void* TransportDIB::memory() const { return shared_memory_.memory(); } diff --git a/app/surface/transport_dib_win.cc b/app/surface/transport_dib_win.cc index f7746e3..f170786 100644 --- a/app/surface/transport_dib_win.cc +++ b/app/surface/transport_dib_win.cc @@ -2,15 +2,74 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include <limits> #include <windows.h> +#include <limits> + #include "app/surface/transport_dib.h" #include "base/logging.h" +#include "base/scoped_handle_win.h" #include "base/scoped_ptr.h" #include "base/sys_info.h" #include "skia/ext/platform_canvas.h" +TransportDIB::Handle TransportDIB::Handle::DupForProcess( + base::ProcessHandle new_owner_process) const { + base::ProcessId new_owner_id = ::GetProcessId(new_owner_process); + if (!new_owner_id) { + LOG(WARNING) << "Could not get process id from handle" + << " handle:" << new_owner_process + << " error:" << ::GetLastError(); + return Handle(); + } + HANDLE owner_process = ::OpenProcess(PROCESS_DUP_HANDLE, FALSE, owner_id_); + if (!owner_process) { + LOG(WARNING) << "Could not open process" + << " id:" << owner_id_ + << " error:" << ::GetLastError(); + return Handle(); + } + ::ScopedHandle scoped_owner_process(owner_process); + HANDLE new_section = NULL; + ::DuplicateHandle(owner_process, section_, + new_owner_process, &new_section, + STANDARD_RIGHTS_REQUIRED | FILE_MAP_ALL_ACCESS, + FALSE, 0); + if (!new_section) { + LOG(WARNING) << "Could not duplicate handle for process" + << " error:" << ::GetLastError(); + return Handle(); + } + return Handle(new_section, new_owner_id, false); +} + +void TransportDIB::Handle::Close() const { + // Do not close this handle if it is invalid, or it has not been duped yet. + if (!is_valid() || should_dup_on_map_) + return; + + // The handle was already duped for a target process, so we should close it. + // The target process may not have been us, so close using + // |::DuplicateHandle|. + HANDLE owner_process = ::OpenProcess(PROCESS_DUP_HANDLE, FALSE, owner_id_); + if (!owner_process) { + LOG(WARNING) << "Could not open process" + << " id:" << owner_id_ + << " error:" << ::GetLastError(); + return; + } + if (!::DuplicateHandle(owner_process, section_, + NULL, NULL, + DUPLICATE_CLOSE_SOURCE, + FALSE, 0)) { + NOTREACHED() << "Handle could not be closed: " << ::GetLastError(); + } +} + +void TransportDIB::ScopedHandle::Close() { + handle_.Close(); +} + TransportDIB::TransportDIB() { } @@ -42,44 +101,73 @@ TransportDIB* TransportDIB::Create(size_t size, uint32 sequence_num) { } // static -TransportDIB* TransportDIB::Map(TransportDIB::Handle handle) { - TransportDIB* dib = new TransportDIB(handle); - if (!dib->shared_memory_.Map(0 /* map whole shared memory segment */)) { - LOG(ERROR) << "Failed to map transport DIB" - << " handle:" << handle - << " error:" << GetLastError(); - delete dib; +TransportDIB* TransportDIB::Map(Handle handle) { + scoped_ptr<TransportDIB> dib(CreateWithHandle(handle)); + if (!dib->Map()) return NULL; - } - - // There doesn't seem to be any way to find the size of the shared memory - // region! GetFileSize indicates that the handle is invalid. Thus, we - // conservatively set the size to the maximum and hope that the renderer - // isn't about to ask us to read off the end of the array. - dib->size_ = std::numeric_limits<size_t>::max(); + return dib.release(); +} - return dib; +// static +TransportDIB* TransportDIB::CreateWithHandle(Handle handle) { + // It is not sufficient to compare the current process ID and the ID in the + // handle here to see if a duplication is required because they will always + // be the same in single process mode. + if (handle.should_dup_on_map()) + handle = handle.DupForProcess(::GetCurrentProcess()); + return new TransportDIB(handle.section()); } +// static bool TransportDIB::is_valid(Handle dib) { - return dib != NULL; + return dib.is_valid(); } skia::PlatformCanvas* TransportDIB::GetPlatformCanvas(int w, int h) { + // This DIB already mapped the file into this process, but PlatformCanvas + // will map it again. + DCHECK(!memory()) << "Mapped file twice in the same process."; + scoped_ptr<skia::PlatformCanvas> canvas(new skia::PlatformCanvas); - if (!canvas->initialize(w, h, true, handle())) + if (!canvas->initialize(w, h, true, shared_memory_.handle())) return NULL; return canvas.release(); } +bool TransportDIB::Map() { + if (memory()) + return true; + + if (!shared_memory_.Map(0 /* map whole shared memory segment */)) { + LOG(ERROR) << "Failed to map transport DIB" + << " handle:" << shared_memory_.handle() + << " error:" << ::GetLastError(); + return false; + } + + // There doesn't seem to be any way to find the size of the shared memory + // region! GetFileSize indicates that the handle is invalid. Thus, we + // conservatively set the size to the maximum and hope that the renderer + // isn't about to ask us to read off the end of the array. + size_ = std::numeric_limits<size_t>::max(); + return true; +} + +TransportDIB::Handle TransportDIB::GetHandleForProcess( + base::ProcessHandle process_handle) const { + return handle().DupForProcess(process_handle); +} + void* TransportDIB::memory() const { return shared_memory_.memory(); } TransportDIB::Handle TransportDIB::handle() const { - return shared_memory_.handle(); + return TransferrableSectionHandle(shared_memory_.handle(), + ::GetCurrentProcessId(), + true); } TransportDIB::Id TransportDIB::id() const { - return Id(shared_memory_.handle(), sequence_num_); + return sequence_num_; } diff --git a/chrome/browser/renderer_host/backing_store.h b/chrome/browser/renderer_host/backing_store.h index 53d0288..4bcbd19 100644 --- a/chrome/browser/renderer_host/backing_store.h +++ b/chrome/browser/renderer_host/backing_store.h @@ -48,7 +48,8 @@ class BackingStore { // DonePaintingToBackingStore(). virtual void PaintToBackingStore( RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, bool* painted_synchronously) = 0; diff --git a/chrome/browser/renderer_host/backing_store_mac.h b/chrome/browser/renderer_host/backing_store_mac.h index 2c75400d..86671d6 100644 --- a/chrome/browser/renderer_host/backing_store_mac.h +++ b/chrome/browser/renderer_host/backing_store_mac.h @@ -26,7 +26,8 @@ class BackingStoreMac : public BackingStore { // BackingStore implementation. virtual void PaintToBackingStore( RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, bool* painted_synchronously); diff --git a/chrome/browser/renderer_host/backing_store_mac.mm b/chrome/browser/renderer_host/backing_store_mac.mm index bea378f..318656c 100644 --- a/chrome/browser/renderer_host/backing_store_mac.mm +++ b/chrome/browser/renderer_host/backing_store_mac.mm @@ -54,7 +54,8 @@ BackingStoreMac::~BackingStoreMac() { void BackingStoreMac::PaintToBackingStore( RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, bool* painted_synchronously) { @@ -64,7 +65,7 @@ void BackingStoreMac::PaintToBackingStore( DCHECK_NE(static_cast<bool>(cg_layer()), static_cast<bool>(cg_bitmap())); - TransportDIB* dib = process->GetTransportDIB(bitmap); + TransportDIB* dib = process->GetTransportDIB(dib_id, dib_handle); if (!dib) return; diff --git a/chrome/browser/renderer_host/backing_store_manager.cc b/chrome/browser/renderer_host/backing_store_manager.cc index 4325f31..745020f 100644 --- a/chrome/browser/renderer_host/backing_store_manager.cc +++ b/chrome/browser/renderer_host/backing_store_manager.cc @@ -193,7 +193,8 @@ BackingStore* BackingStoreManager::GetBackingStore( void BackingStoreManager::PrepareBackingStore( RenderWidgetHost* host, const gfx::Size& backing_store_size, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, bool* needs_full_paint, @@ -217,8 +218,11 @@ void BackingStoreManager::PrepareBackingStore( backing_store = CreateBackingStore(host, backing_store_size); } - backing_store->PaintToBackingStore(host->process(), bitmap, - bitmap_rect, copy_rects, + backing_store->PaintToBackingStore(host->process(), + dib_id, + dib_handle, + bitmap_rect, + copy_rects, painted_synchronously); } diff --git a/chrome/browser/renderer_host/backing_store_manager.h b/chrome/browser/renderer_host/backing_store_manager.h index 1ab78cc..48e7924 100644 --- a/chrome/browser/renderer_host/backing_store_manager.h +++ b/chrome/browser/renderer_host/backing_store_manager.h @@ -51,7 +51,8 @@ class BackingStoreManager { static void PrepareBackingStore( RenderWidgetHost* host, const gfx::Size& backing_store_size, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, bool* needs_full_paint, diff --git a/chrome/browser/renderer_host/backing_store_proxy.cc b/chrome/browser/renderer_host/backing_store_proxy.cc index 3d2fd8e..e5609e5 100644 --- a/chrome/browser/renderer_host/backing_store_proxy.cc +++ b/chrome/browser/renderer_host/backing_store_proxy.cc @@ -12,10 +12,6 @@ #include "chrome/common/render_messages.h" #include "gfx/rect.h" -#if defined(OS_WIN) -#include <windows.h> -#endif - BackingStoreProxy::BackingStoreProxy(RenderWidgetHost* widget, const gfx::Size& size, GpuProcessHostUIShim* process_shim, @@ -33,21 +29,16 @@ BackingStoreProxy::~BackingStoreProxy() { void BackingStoreProxy::PaintToBackingStore( RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, bool* painted_synchronously) { DCHECK(!waiting_for_paint_ack_); - - base::ProcessId process_id; -#if defined(OS_WIN) - process_id = ::GetProcessId(process->GetHandle()); -#elif defined(OS_POSIX) - process_id = process->GetHandle(); -#endif + TransportDIB::ScopedHandle scoped_dib_handle(dib_handle); if (process_shim_->Send(new GpuMsg_PaintToBackingStore( - routing_id_, process_id, bitmap, bitmap_rect, copy_rects))) { + routing_id_, scoped_dib_handle.release(), bitmap_rect, copy_rects))) { // Message sent successfully, so the caller can not destroy the // TransportDIB. OnDonePaintingToBackingStore will free it later. *painted_synchronously = false; diff --git a/chrome/browser/renderer_host/backing_store_proxy.h b/chrome/browser/renderer_host/backing_store_proxy.h index 610f7ef..a9a3c3e 100644 --- a/chrome/browser/renderer_host/backing_store_proxy.h +++ b/chrome/browser/renderer_host/backing_store_proxy.h @@ -21,7 +21,8 @@ class BackingStoreProxy : public BackingStore, // BackingStore implementation. virtual void PaintToBackingStore(RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, bool* painted_synchronously); diff --git a/chrome/browser/renderer_host/backing_store_win.cc b/chrome/browser/renderer_host/backing_store_win.cc index 94d9cbf..b8385d3 100644 --- a/chrome/browser/renderer_host/backing_store_win.cc +++ b/chrome/browser/renderer_host/backing_store_win.cc @@ -69,7 +69,8 @@ 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) { @@ -114,10 +115,12 @@ size_t BackingStoreWin::MemorySize() { void BackingStoreWin::PaintToBackingStore( RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, bool* painted_synchronously) { + TransportDIB::ScopedHandle scoped_dib_handle(dib_handle); // Our paints are always synchronous and the TransportDIB can be freed when // we're done (even on error). *painted_synchronously = true; @@ -132,7 +135,8 @@ void BackingStoreWin::PaintToBackingStore( original_bitmap_ = SelectObject(hdc_, backing_store_dib_); } - TransportDIB* dib = process->GetTransportDIB(bitmap); + TransportDIB* dib = process->GetTransportDIB(dib_id, + scoped_dib_handle.release()); if (!dib) return; diff --git a/chrome/browser/renderer_host/backing_store_win.h b/chrome/browser/renderer_host/backing_store_win.h index 221e0aa..a99c835 100644 --- a/chrome/browser/renderer_host/backing_store_win.h +++ b/chrome/browser/renderer_host/backing_store_win.h @@ -24,7 +24,8 @@ class BackingStoreWin : public BackingStore { // BackingStore implementation. virtual size_t MemorySize(); virtual void PaintToBackingStore(RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, bool* painted_synchronously); diff --git a/chrome/browser/renderer_host/backing_store_x.cc b/chrome/browser/renderer_host/backing_store_x.cc index a79a8fc..34db658 100644 --- a/chrome/browser/renderer_host/backing_store_x.cc +++ b/chrome/browser/renderer_host/backing_store_x.cc @@ -158,10 +158,12 @@ void BackingStoreX::PaintRectWithoutXrender( void BackingStoreX::PaintToBackingStore( RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, bool* painted_synchronously) { + TransportDIB::ScopedHandle scoped_dib_handle(dib_handle); // Our paints are always synchronous and the caller can free the TransportDIB // when we're done, even on error. *painted_synchronously = true; @@ -179,7 +181,8 @@ void BackingStoreX::PaintToBackingStore( height <= 0 || height > kMaxVideoLayerSize) return; - TransportDIB* dib = process->GetTransportDIB(bitmap); + TransportDIB* dib = process->GetTransportDIB(dib_id, + scoped_dib_handle.release()); if (!dib) return; diff --git a/chrome/browser/renderer_host/backing_store_x.h b/chrome/browser/renderer_host/backing_store_x.h index 283c19f..2fce04f 100644 --- a/chrome/browser/renderer_host/backing_store_x.h +++ b/chrome/browser/renderer_host/backing_store_x.h @@ -51,7 +51,8 @@ class BackingStoreX : public BackingStore { virtual size_t MemorySize(); virtual void PaintToBackingStore( RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, bool* painted_synchronously); diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc index 5e254e5..aed96e0 100644 --- a/chrome/browser/renderer_host/browser_render_process_host.cc +++ b/chrome/browser/renderer_host/browser_render_process_host.cc @@ -778,23 +778,20 @@ bool BrowserRenderProcessHost::SendWithTimeout(IPC::Message* msg, // This is a platform specific function for mapping a transport DIB given its id TransportDIB* BrowserRenderProcessHost::MapTransportDIB( - TransportDIB::Id dib_id) { -#if defined(OS_WIN) - // On Windows we need to duplicate the handle from the remote process - HANDLE section = win_util::GetSectionFromProcess( - dib_id.handle, GetHandle(), false /* read write */); - return TransportDIB::Map(section); -#elif defined(OS_MACOSX) + TransportDIB::Id dib_id, TransportDIB::Handle dib_handle) { + TransportDIB::ScopedHandle scoped_handle(dib_handle); +#if defined(OS_MACOSX) // On OSX, the browser allocates all DIBs and keeps a file descriptor around // for each. return widget_helper_->MapTransportDIB(dib_id); -#elif defined(OS_POSIX) - return TransportDIB::Map(dib_id); -#endif // defined(OS_POSIX) +#else + return TransportDIB::Map(scoped_handle.release()); +#endif } TransportDIB* BrowserRenderProcessHost::GetTransportDIB( - TransportDIB::Id dib_id) { + TransportDIB::Id dib_id, TransportDIB::Handle dib_handle) { + TransportDIB::ScopedHandle scoped_handle(dib_handle); const std::map<TransportDIB::Id, TransportDIB*>::iterator i = cached_dibs_.find(dib_id); if (i != cached_dibs_.end()) { @@ -802,7 +799,7 @@ TransportDIB* BrowserRenderProcessHost::GetTransportDIB( return i->second; } - TransportDIB* dib = MapTransportDIB(dib_id); + TransportDIB* dib = MapTransportDIB(dib_id, scoped_handle.release()); if (!dib) return NULL; diff --git a/chrome/browser/renderer_host/browser_render_process_host.h b/chrome/browser/renderer_host/browser_render_process_host.h index 9983c84..16a5cac 100644 --- a/chrome/browser/renderer_host/browser_render_process_host.h +++ b/chrome/browser/renderer_host/browser_render_process_host.h @@ -78,7 +78,8 @@ class BrowserRenderProcessHost : public RenderProcessHost, virtual bool FastShutdownIfPossible(); virtual bool SendWithTimeout(IPC::Message* msg, int timeout_ms); virtual base::ProcessHandle GetHandle(); - virtual TransportDIB* GetTransportDIB(TransportDIB::Id dib_id); + virtual TransportDIB* GetTransportDIB(TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle); // IPC::Channel::Sender via RenderProcessHost. virtual bool Send(IPC::Message* msg); @@ -185,8 +186,10 @@ class BrowserRenderProcessHost : public RenderProcessHost, MAX_MAPPED_TRANSPORT_DIBS = 3, }; - // Map a transport DIB from its Id and return it. Returns NULL on error. - TransportDIB* MapTransportDIB(TransportDIB::Id dib_id); + // Map a transport DIB from its handle. On Mac, the ID is necessary to find + // the appropriate file descriptor. Returns NULL on error. + TransportDIB* MapTransportDIB(TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle); void ClearTransportDIBCache(); // This is used to clear our cache five seconds after the last use. diff --git a/chrome/browser/renderer_host/mock_render_process_host.cc b/chrome/browser/renderer_host/mock_render_process_host.cc index a2ccfa0..e39d5b7 100644 --- a/chrome/browser/renderer_host/mock_render_process_host.cc +++ b/chrome/browser/renderer_host/mock_render_process_host.cc @@ -98,20 +98,17 @@ bool MockRenderProcessHost::Send(IPC::Message* msg) { return true; } -TransportDIB* MockRenderProcessHost::GetTransportDIB(TransportDIB::Id dib_id) { +TransportDIB* MockRenderProcessHost::GetTransportDIB( + TransportDIB::Id dib_id, TransportDIB::Handle dib_handle) { + TransportDIB::ScopedHandle scoped_dib_handle(dib_handle); if (transport_dib_) return transport_dib_; -#if defined(OS_WIN) - HANDLE duped; - DuplicateHandle(GetCurrentProcess(), dib_id.handle, GetCurrentProcess(), - &duped, 0, TRUE, DUPLICATE_SAME_ACCESS); - transport_dib_ = TransportDIB::Map(duped); -#elif defined(OS_MACOSX) +#if defined(OS_MACOSX) // On Mac, TransportDIBs are always created in the browser, so we cannot map // one from a dib_id. transport_dib_ = TransportDIB::Create(100 * 100 * 4, 0); -#elif defined(OS_POSIX) - transport_dib_ = TransportDIB::Map(dib_id); +#else + transport_dib_ = TransportDIB::Map(scoped_dib_handle.release()); #endif return transport_dib_; diff --git a/chrome/browser/renderer_host/mock_render_process_host.h b/chrome/browser/renderer_host/mock_render_process_host.h index 232725b..494a45c 100644 --- a/chrome/browser/renderer_host/mock_render_process_host.h +++ b/chrome/browser/renderer_host/mock_render_process_host.h @@ -53,7 +53,8 @@ class MockRenderProcessHost : public RenderProcessHost { virtual bool SendWithTimeout(IPC::Message* msg, int timeout_ms); virtual base::ProcessHandle GetHandle(); - virtual TransportDIB* GetTransportDIB(TransportDIB::Id dib_id); + virtual TransportDIB* GetTransportDIB(TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle); // IPC::Channel::Sender via RenderProcessHost. virtual bool Send(IPC::Message* msg); diff --git a/chrome/browser/renderer_host/render_process_host.h b/chrome/browser/renderer_host/render_process_host.h index a5fd378..38cb5f2 100644 --- a/chrome/browser/renderer_host/render_process_host.h +++ b/chrome/browser/renderer_host/render_process_host.h @@ -216,12 +216,12 @@ class RenderProcessHost : public IPC::Channel::Sender, // Transport DIB functions --------------------------------------------------- - // Return the TransportDIB for the given id. On Linux, this can involve - // mapping shared memory. On Mac, the shared memory is created in the browser - // process and the cached metadata is returned. On Windows, this involves - // duplicating the handle from the remote process. The RenderProcessHost - // still owns the returned DIB. - virtual TransportDIB* GetTransportDIB(TransportDIB::Id dib_id) = 0; + // Return the TransportDIB for the given id. On Windows and Linux, this may + // involve mapping the TransportDIB. On Mac, the shared memory is created in + // the browser process and the cached metadata is returned. The + // RenderProcessHost still owns the returned DIB. + virtual TransportDIB* GetTransportDIB(TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle) = 0; // Static management functions ----------------------------------------------- diff --git a/chrome/browser/renderer_host/render_widget_host.cc b/chrome/browser/renderer_host/render_widget_host.cc index a6efd58..bf98b7d 100644 --- a/chrome/browser/renderer_host/render_widget_host.cc +++ b/chrome/browser/renderer_host/render_widget_host.cc @@ -763,6 +763,7 @@ void RenderWidgetHost::OnMsgPaintAtSizeAck(int tag, const gfx::Size& size) { void RenderWidgetHost::OnMsgUpdateRect( const ViewHostMsg_UpdateRect_Params& params) { + TransportDIB::ScopedHandle dib_handle(params.dib_handle); TimeTicks paint_start = TimeTicks::Now(); if (paint_observer_.get()) @@ -797,7 +798,8 @@ void RenderWidgetHost::OnMsgUpdateRect( if (!is_gpu_rendering_active_) { const size_t size = params.bitmap_rect.height() * params.bitmap_rect.width() * 4; - TransportDIB* dib = process_->GetTransportDIB(params.bitmap); + TransportDIB* dib = process_->GetTransportDIB(params.dib_id, + dib_handle.release()); // If gpu process does painting, scroll_rect and copy_rects are always empty // and backing store is never used. @@ -816,8 +818,11 @@ 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, + PaintBackingStoreRect(params.dib_id, + params.dib_handle, + params.bitmap_rect, + params.copy_rects, + params.view_size, &painted_synchronously); } } @@ -881,9 +886,10 @@ void RenderWidgetHost::OnMsgCreateVideo(const gfx::Size& size) { Send(new ViewMsg_CreateVideo_ACK(routing_id_, -1)); } -void RenderWidgetHost::OnMsgUpdateVideo(TransportDIB::Id bitmap, +void RenderWidgetHost::OnMsgUpdateVideo(TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect) { - PaintVideoLayer(bitmap, bitmap_rect); + PaintVideoLayer(dib_id, dib_handle, bitmap_rect); // TODO(scherkus): support actual video ids! Send(new ViewMsg_UpdateVideo_ACK(routing_id_, -1)); @@ -1049,9 +1055,10 @@ void RenderWidgetHost::OnAcceleratedSurfaceSetTransportDIB( int32 width, int32 height, TransportDIB::Handle transport_dib) { + TransportDIB::ScopedHandle scoped_dib_handle(transport_dib); if (view_) { view_->AcceleratedSurfaceSetTransportDIB(window, width, height, - transport_dib); + scoped_dib_handle.release()); } } @@ -1085,7 +1092,8 @@ void RenderWidgetHost::OnMsgDestroyPluginContainer(gfx::PluginWindowHandle id) { #endif void RenderWidgetHost::PaintBackingStoreRect( - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, const gfx::Size& view_size, @@ -1107,8 +1115,13 @@ void RenderWidgetHost::PaintBackingStoreRect( } bool needs_full_paint = false; - BackingStoreManager::PrepareBackingStore(this, view_size, bitmap, bitmap_rect, - copy_rects, &needs_full_paint, + BackingStoreManager::PrepareBackingStore(this, + view_size, + dib_id, + dib_handle, + bitmap_rect, + copy_rects, + &needs_full_paint, painted_synchronously); if (needs_full_paint) { repaint_start_time_ = TimeTicks::Now(); @@ -1137,12 +1150,13 @@ void RenderWidgetHost::ScrollBackingStoreRect(int dx, int dy, backing_store->ScrollBackingStore(dx, dy, clip_rect, view_size); } -void RenderWidgetHost::PaintVideoLayer(TransportDIB::Id bitmap, +void RenderWidgetHost::PaintVideoLayer(TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect) { if (is_hidden_ || !video_layer_.get()) return; - video_layer_->CopyTransportDIB(process(), bitmap, bitmap_rect); + video_layer_->CopyTransportDIB(process(), dib_id, dib_handle, bitmap_rect); // Don't update the view if we're hidden or if the view has been destroyed. if (is_hidden_ || !view_) diff --git a/chrome/browser/renderer_host/render_widget_host.h b/chrome/browser/renderer_host/render_widget_host.h index 6c3fed7..26d1c2c 100644 --- a/chrome/browser/renderer_host/render_widget_host.h +++ b/chrome/browser/renderer_host/render_widget_host.h @@ -480,7 +480,9 @@ class RenderWidgetHost : public IPC::Channel::Listener, void OnMsgPaintAtSizeAck(int tag, const gfx::Size& size); void OnMsgUpdateRect(const ViewHostMsg_UpdateRect_Params& params); void OnMsgCreateVideo(const gfx::Size& size); - void OnMsgUpdateVideo(TransportDIB::Id bitmap, const gfx::Rect& bitmap_rect); + void OnMsgUpdateVideo(TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, + const gfx::Rect& bitmap_rect); void OnMsgDestroyVideo(); void OnMsgInputEventAck(const IPC::Message& message); virtual void OnMsgFocus(); @@ -523,7 +525,8 @@ class RenderWidgetHost : public IPC::Channel::Listener, // 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 DIB can't // be freed (it will be the backing store's job to free it later). - void PaintBackingStoreRect(TransportDIB::Id bitmap, + void PaintBackingStoreRect(TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, const gfx::Size& view_size, @@ -538,7 +541,8 @@ class RenderWidgetHost : public IPC::Channel::Listener, // Paints the entire given bitmap into the current video layer, if it exists. // |bitmap_rect| specifies the destination size and absolute location of the // bitmap on the backing store. - void PaintVideoLayer(TransportDIB::Id bitmap, + void PaintVideoLayer(TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect); // Called by OnMsgInputEventAck() to process a keyboard event ack message. diff --git a/chrome/browser/renderer_host/render_widget_host_unittest.cc b/chrome/browser/renderer_host/render_widget_host_unittest.cc index 1c5bb17..02b3bd8 100644 --- a/chrome/browser/renderer_host/render_widget_host_unittest.cc +++ b/chrome/browser/renderer_host/render_widget_host_unittest.cc @@ -84,7 +84,8 @@ void RenderWidgetHostProcess::InitUpdateRectParams( if (!current_update_buf_) current_update_buf_ = TransportDIB::Create(pixel_size, 0); - params->bitmap = current_update_buf_->id(); + params->dib_id = current_update_buf_->id(); + params->dib_handle = current_update_buf_->handle(); params->bitmap_rect = gfx::Rect(0, 0, w, h); params->dx = 0; params->dy = 0; diff --git a/chrome/browser/renderer_host/test/test_backing_store.cc b/chrome/browser/renderer_host/test/test_backing_store.cc index d90c7fd..01dc7d8 100644 --- a/chrome/browser/renderer_host/test/test_backing_store.cc +++ b/chrome/browser/renderer_host/test/test_backing_store.cc @@ -14,7 +14,8 @@ TestBackingStore::~TestBackingStore() { void TestBackingStore::PaintToBackingStore( RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, bool* painted_synchronously) { diff --git a/chrome/browser/renderer_host/test/test_backing_store.h b/chrome/browser/renderer_host/test/test_backing_store.h index f9db76e..d532dc1 100644 --- a/chrome/browser/renderer_host/test/test_backing_store.h +++ b/chrome/browser/renderer_host/test/test_backing_store.h @@ -16,7 +16,8 @@ class TestBackingStore : public BackingStore { // BackingStore implementation. virtual void PaintToBackingStore(RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects, bool* painted_synchronously); diff --git a/chrome/browser/renderer_host/video_layer.h b/chrome/browser/renderer_host/video_layer.h index 0d7d4f8..4bcc245 100644 --- a/chrome/browser/renderer_host/video_layer.h +++ b/chrome/browser/renderer_host/video_layer.h @@ -26,12 +26,13 @@ class VideoLayer { RenderWidgetHost* render_widget_host() const { return render_widget_host_; } const gfx::Size& size() { return size_; } - // Copy the incoming bitmap into this video layer. |bitmap| contains YUV + // Copy the incoming bitmap into this video layer. The given DIB contains YUV // pixel data in YV12 format and must be the same dimensions as this video // layer. |bitmap_rect| specifies the absolute position and destination size // of the bitmap on the backing store. virtual void CopyTransportDIB(RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect) = 0; protected: diff --git a/chrome/browser/renderer_host/video_layer_proxy.cc b/chrome/browser/renderer_host/video_layer_proxy.cc index 3df1d25..fb4cfe9 100644 --- a/chrome/browser/renderer_host/video_layer_proxy.cc +++ b/chrome/browser/renderer_host/video_layer_proxy.cc @@ -5,7 +5,6 @@ #include "chrome/browser/renderer_host/video_layer_proxy.h" #include "chrome/browser/gpu_process_host_ui_shim.h" -#include "chrome/browser/renderer_host/render_process_host.h" #include "chrome/common/gpu_messages.h" #include "gfx/rect.h" @@ -24,17 +23,12 @@ VideoLayerProxy::~VideoLayerProxy() { } void VideoLayerProxy::CopyTransportDIB(RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect) { - base::ProcessId process_id; -#if defined(OS_WIN) - process_id = ::GetProcessId(process->GetHandle()); -#elif defined(OS_POSIX) - process_id = process->GetHandle(); -#endif - + TransportDIB::ScopedHandle scoped_dib_handle(dib_handle); if (process_shim_->Send(new GpuMsg_PaintToVideoLayer( - routing_id_, process_id, bitmap, bitmap_rect))) { + routing_id_, scoped_dib_handle.release(), bitmap_rect))) { } else { // TODO(scherkus): what to do ?!?! } diff --git a/chrome/browser/renderer_host/video_layer_proxy.h b/chrome/browser/renderer_host/video_layer_proxy.h index 4a4cc71..4d3b1f2f 100644 --- a/chrome/browser/renderer_host/video_layer_proxy.h +++ b/chrome/browser/renderer_host/video_layer_proxy.h @@ -20,7 +20,8 @@ class VideoLayerProxy : public VideoLayer, public IPC::Channel::Listener { // VideoLayer implementation. virtual void CopyTransportDIB(RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect); // IPC::Channel::Listener implementation. diff --git a/chrome/browser/renderer_host/video_layer_x.cc b/chrome/browser/renderer_host/video_layer_x.cc index 09891b3..a8a878b 100644 --- a/chrome/browser/renderer_host/video_layer_x.cc +++ b/chrome/browser/renderer_host/video_layer_x.cc @@ -44,8 +44,10 @@ VideoLayerX::~VideoLayerX() { } void VideoLayerX::CopyTransportDIB(RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect) { + TransportDIB::ScopedHandle scoped_dib_handle(dib_handle); if (!display_) return; @@ -76,7 +78,8 @@ void VideoLayerX::CopyTransportDIB(RenderProcessHost* process, rgb_frame_size_ = new_rgb_frame_size; } - TransportDIB* dib = process->GetTransportDIB(bitmap); + TransportDIB* dib = process->GetTransportDIB(dib_id, + scoped_dib_handle.release()); if (!dib) return; diff --git a/chrome/browser/renderer_host/video_layer_x.h b/chrome/browser/renderer_host/video_layer_x.h index a12c7b4..8c3a66f 100644 --- a/chrome/browser/renderer_host/video_layer_x.h +++ b/chrome/browser/renderer_host/video_layer_x.h @@ -20,7 +20,8 @@ class VideoLayerX : public VideoLayer { // VideoLayer implementation. virtual void CopyTransportDIB(RenderProcessHost* process, - TransportDIB::Id bitmap, + TransportDIB::Id dib_id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect); // Copy from the server-side video layer to the target window. diff --git a/chrome/browser/tab_contents/thumbnail_generator.cc b/chrome/browser/tab_contents/thumbnail_generator.cc index 520faf4..34ddf42 100644 --- a/chrome/browser/tab_contents/thumbnail_generator.cc +++ b/chrome/browser/tab_contents/thumbnail_generator.cc @@ -11,6 +11,7 @@ #include "base/time.h" #include "build/build_config.h" #include "chrome/browser/renderer_host/backing_store.h" +#include "chrome/browser/renderer_host/render_process_host.h" #include "chrome/browser/renderer_host/render_view_host.h" #include "chrome/browser/tab_contents/tab_contents.h" #include "chrome/common/notification_service.h" @@ -208,7 +209,8 @@ void ThumbnailGenerator::AskForSnapshot(RenderWidgetHost* renderer, } renderer->PaintAtSize( - thumbnail_dib->handle(), sequence_num, page_size, desired_size); + thumbnail_dib->GetHandleForProcess(renderer->process()->GetHandle()), + sequence_num, page_size, desired_size); } SkBitmap ThumbnailGenerator::GetThumbnailForRenderer( @@ -299,7 +301,7 @@ void ThumbnailGenerator::WidgetDidReceivePaintAtSizeAck( if (item != callback_map_.end()) { TransportDIB* dib = item->second->thumbnail_dib.get(); DCHECK(dib); - if (!dib) { + if (!dib || !dib->Map()) { return; } diff --git a/chrome/common/common_param_traits.h b/chrome/common/common_param_traits.h index 11c9749..2fed4b3 100644 --- a/chrome/common/common_param_traits.h +++ b/chrome/common/common_param_traits.h @@ -12,6 +12,10 @@ #define CHROME_COMMON_COMMON_PARAM_TRAITS_H_ #pragma once +#if defined(OS_WIN) +#include <windows.h> +#endif + #include "app/surface/transport_dib.h" #include "base/file_util.h" #include "base/ref_counted.h" @@ -236,22 +240,33 @@ struct ParamTraits<webkit_glue::WebApplicationInfo> { #if defined(OS_WIN) -template<> -struct ParamTraits<TransportDIB::Id> { - typedef TransportDIB::Id param_type; +template <> +struct ParamTraits<TransportDIB::Handle> { + typedef TransportDIB::Handle param_type; static void Write(Message* m, const param_type& p) { - WriteParam(m, p.handle); - WriteParam(m, p.sequence_num); + WriteParam(m, p.section()); + WriteParam(m, p.owner_id()); + WriteParam(m, p.should_dup_on_map()); } static bool Read(const Message* m, void** iter, param_type* r) { - return (ReadParam(m, iter, &r->handle) && - ReadParam(m, iter, &r->sequence_num)); + HANDLE section; + base::ProcessId owner_id; + bool should_dup_on_map; + bool success = ReadParam(m, iter, §ion) && + ReadParam(m, iter, &owner_id) && + ReadParam(m, iter, &should_dup_on_map); + if (success) { + *r = TransportDIB::Handle(section, owner_id, should_dup_on_map); + } + return success; } static void Log(const param_type& p, std::string* l) { - l->append("TransportDIB("); - LogParam(p.handle, l); + l->append("TransportDIB::Handle("); + LogParam(p.section(), l); + l->append(", "); + LogParam(p.owner_id(), l); l->append(", "); - LogParam(p.sequence_num, l); + LogParam(p.should_dup_on_map(), l); l->append(")"); } }; diff --git a/chrome/common/gpu_messages_internal.h b/chrome/common/gpu_messages_internal.h index 184d8f4..deb48f8 100644 --- a/chrome/common/gpu_messages_internal.h +++ b/chrome/common/gpu_messages_internal.h @@ -74,13 +74,11 @@ IPC_BEGIN_MESSAGES(Gpu) // Updates the backing store with the given bitmap. The GPU process will send // back a GpuHostMsg_PaintToBackingStore_ACK after the paint is complete to // let the caller know the TransportDIB can be freed or reused. - IPC_MESSAGE_ROUTED4(GpuMsg_PaintToBackingStore, - base::ProcessId, /* process */ - TransportDIB::Id, /* bitmap */ + IPC_MESSAGE_ROUTED3(GpuMsg_PaintToBackingStore, + TransportDIB::Handle, /* bitmap_handle */ gfx::Rect, /* bitmap_rect */ std::vector<gfx::Rect>) /* copy_rects */ - IPC_MESSAGE_ROUTED4(GpuMsg_ScrollBackingStore, int, /* dx */ int, /* dy */ @@ -95,9 +93,8 @@ IPC_BEGIN_MESSAGES(Gpu) // Updates the video layer with the given YUV data. The GPU process will send // back a GpuHostMsg_PaintToVideoLayer_ACK after the paint is complete to // let the caller know the TransportDIB can be freed or reused. - IPC_MESSAGE_ROUTED3(GpuMsg_PaintToVideoLayer, - base::ProcessId, /* process */ - TransportDIB::Id, /* bitmap */ + IPC_MESSAGE_ROUTED2(GpuMsg_PaintToVideoLayer, + TransportDIB::Handle, /* bitmap_handle */ gfx::Rect) /* bitmap_rect */ IPC_END_MESSAGES(Gpu) diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h index 75eacdf..70edb54 100644 --- a/chrome/common/render_messages_internal.h +++ b/chrome/common/render_messages_internal.h @@ -1276,8 +1276,9 @@ IPC_BEGIN_MESSAGES(ViewHost) // Sent to create, update and destroy video layers. IPC_MESSAGE_ROUTED1(ViewHostMsg_CreateVideo, gfx::Size /* size */) - IPC_MESSAGE_ROUTED2(ViewHostMsg_UpdateVideo, - TransportDIB::Id /* bitmap */, + IPC_MESSAGE_ROUTED3(ViewHostMsg_UpdateVideo, + TransportDIB::Id /* bitmap_id */, + TransportDIB::Handle /* bitmap_handle */, gfx::Rect /* bitmap_rect */) IPC_MESSAGE_ROUTED0(ViewHostMsg_DestroyVideo) diff --git a/chrome/common/render_messages_params.cc b/chrome/common/render_messages_params.cc index 59802b4..73f5ea2 100644 --- a/chrome/common/render_messages_params.cc +++ b/chrome/common/render_messages_params.cc @@ -38,11 +38,11 @@ ViewHostMsg_FrameNavigate_Params::~ViewHostMsg_FrameNavigate_Params() { } ViewHostMsg_UpdateRect_Params::ViewHostMsg_UpdateRect_Params() - : dx(0), + : dib_id(0), + dib_handle(TransportDIB::DefaultHandleValue()), + dx(0), dy(0), flags(0) { - // On windows, bitmap is of type "struct HandleAndSequenceNum" - memset(&bitmap, 0, sizeof(bitmap)); } ViewHostMsg_UpdateRect_Params::~ViewHostMsg_UpdateRect_Params() { @@ -835,7 +835,8 @@ void ParamTraits<ViewHostMsg_FrameNavigate_Params>::Log(const param_type& p, void ParamTraits<ViewHostMsg_UpdateRect_Params>::Write( Message* m, const param_type& p) { - WriteParam(m, p.bitmap); + WriteParam(m, p.dib_id); + WriteParam(m, p.dib_handle); WriteParam(m, p.bitmap_rect); WriteParam(m, p.dx); WriteParam(m, p.dy); @@ -849,7 +850,8 @@ void ParamTraits<ViewHostMsg_UpdateRect_Params>::Write( bool ParamTraits<ViewHostMsg_UpdateRect_Params>::Read( const Message* m, void** iter, param_type* p) { return - ReadParam(m, iter, &p->bitmap) && + ReadParam(m, iter, &p->dib_id) && + ReadParam(m, iter, &p->dib_handle) && ReadParam(m, iter, &p->bitmap_rect) && ReadParam(m, iter, &p->dx) && ReadParam(m, iter, &p->dy) && @@ -863,7 +865,9 @@ bool ParamTraits<ViewHostMsg_UpdateRect_Params>::Read( void ParamTraits<ViewHostMsg_UpdateRect_Params>::Log(const param_type& p, std::string* l) { l->append("("); - LogParam(p.bitmap, l); + LogParam(p.dib_id, l); + l->append(", "); + LogParam(p.dib_handle, l); l->append(", "); LogParam(p.bitmap_rect, l); l->append(", "); diff --git a/chrome/common/render_messages_params.h b/chrome/common/render_messages_params.h index 2eeff70..211de1a 100644 --- a/chrome/common/render_messages_params.h +++ b/chrome/common/render_messages_params.h @@ -321,9 +321,12 @@ struct ViewHostMsg_UpdateRect_Params { ViewHostMsg_UpdateRect_Params(); ~ViewHostMsg_UpdateRect_Params(); - // The bitmap to be painted into the view at the locations specified by - // update_rects. - TransportDIB::Id bitmap; + // The ID of the bitmap to be painted into the view at the locations specified + // by update_rects. + TransportDIB::Id dib_id; + + // The handle to the bitmap, in case it needs to be mapped on the browser. + TransportDIB::Handle dib_handle; // The position and size of the bitmap. gfx::Rect bitmap_rect; diff --git a/chrome/gpu/gpu_backing_store_glx.cc b/chrome/gpu/gpu_backing_store_glx.cc index 916a1db..ba5fa20 100644 --- a/chrome/gpu/gpu_backing_store_glx.cc +++ b/chrome/gpu/gpu_backing_store_glx.cc @@ -60,11 +60,10 @@ void GpuBackingStoreGLX::OnChannelError() { } void GpuBackingStoreGLX::OnPaintToBackingStore( - base::ProcessId source_process_id, - TransportDIB::Id id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects) { - scoped_ptr<TransportDIB> dib(TransportDIB::Map(id)); + scoped_ptr<TransportDIB> dib(TransportDIB::Map(dib_handle)); view_->BindContext(); scoped_ptr<skia::PlatformCanvas> canvas( diff --git a/chrome/gpu/gpu_backing_store_glx.h b/chrome/gpu/gpu_backing_store_glx.h index 14b1f5f..af1ced5 100644 --- a/chrome/gpu/gpu_backing_store_glx.h +++ b/chrome/gpu/gpu_backing_store_glx.h @@ -36,8 +36,7 @@ class GpuBackingStoreGLX : public IPC::Channel::Listener { private: // Message handlers. - void OnPaintToBackingStore(base::ProcessId source_process_id, - TransportDIB::Id id, + void OnPaintToBackingStore(TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects); void OnScrollBackingStore(int dx, int dy, diff --git a/chrome/gpu/gpu_backing_store_win.cc b/chrome/gpu/gpu_backing_store_win.cc index d5af441..2b22369 100644 --- a/chrome/gpu/gpu_backing_store_win.cc +++ b/chrome/gpu/gpu_backing_store_win.cc @@ -116,21 +116,10 @@ void GpuBackingStoreWin::OnChannelError() { } void GpuBackingStoreWin::OnPaintToBackingStore( - base::ProcessId source_process_id, - TransportDIB::Id id, + TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects) { - HANDLE source_process_handle = OpenProcess(PROCESS_ALL_ACCESS, - FALSE, source_process_id); - CHECK(source_process_handle); - - // On Windows we need to duplicate the handle from the remote process. - // See BrowserRenderProcessHost::MapTransportDIB for how to do this on other - // platforms. - HANDLE section = win_util::GetSectionFromProcess( - id.handle, source_process_handle, false /* read write */); - CHECK(section); - scoped_ptr<TransportDIB> dib(TransportDIB::Map(section)); + scoped_ptr<TransportDIB> dib(TransportDIB::Map(dib_handle)); CHECK(dib.get()); if (!backing_store_dib_) { @@ -168,7 +157,6 @@ void GpuBackingStoreWin::OnPaintToBackingStore( view_->InvalidateRect(&paint_rect.ToRECT()); } - CloseHandle(source_process_handle); gpu_thread_->Send(new GpuHostMsg_PaintToBackingStore_ACK(routing_id_)); } diff --git a/chrome/gpu/gpu_backing_store_win.h b/chrome/gpu/gpu_backing_store_win.h index fc4bfd9..2899e93 100644 --- a/chrome/gpu/gpu_backing_store_win.h +++ b/chrome/gpu/gpu_backing_store_win.h @@ -42,8 +42,7 @@ class GpuBackingStoreWin : public IPC::Channel::Listener { private: // Message handlers. - void OnPaintToBackingStore(base::ProcessId source_process_id, - TransportDIB::Id id, + void OnPaintToBackingStore(TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect, const std::vector<gfx::Rect>& copy_rects); void OnScrollBackingStore(int dx, int dy, diff --git a/chrome/gpu/gpu_video_layer_glx.cc b/chrome/gpu/gpu_video_layer_glx.cc index d0578ad..dffa35e 100644 --- a/chrome/gpu/gpu_video_layer_glx.cc +++ b/chrome/gpu/gpu_video_layer_glx.cc @@ -251,9 +251,9 @@ void GpuVideoLayerGLX::OnChannelError() { NOTIMPLEMENTED(); } -void GpuVideoLayerGLX::OnPaintToVideoLayer(base::ProcessId source_process_id, - TransportDIB::Id id, +void GpuVideoLayerGLX::OnPaintToVideoLayer(TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect) { + TransportDIB::ScopedHandle scoped_dib_handle(dib_handle); // TODO(scherkus): |native_size_| is set in constructor, so perhaps this check // should be a DCHECK(). const int width = native_size_.width(); @@ -264,7 +264,7 @@ void GpuVideoLayerGLX::OnPaintToVideoLayer(base::ProcessId source_process_id, height <= 0 || height > kMaxVideoLayerSize) return; - TransportDIB* dib = TransportDIB::Map(id); + TransportDIB* dib = TransportDIB::Map(scoped_dib_handle.release()); if (!dib) return; diff --git a/chrome/gpu/gpu_video_layer_glx.h b/chrome/gpu/gpu_video_layer_glx.h index 4b3497f..89d00b1 100644 --- a/chrome/gpu/gpu_video_layer_glx.h +++ b/chrome/gpu/gpu_video_layer_glx.h @@ -40,8 +40,7 @@ class GpuVideoLayerGLX : public IPC::Channel::Listener { private: // Message handlers. - void OnPaintToVideoLayer(base::ProcessId source_process_id, - TransportDIB::Id id, + void OnPaintToVideoLayer(TransportDIB::Handle dib_handle, const gfx::Rect& bitmap_rect); // Calculates vertices for |object| relative to |world|, where |world| is diff --git a/chrome/plugin/webplugin_proxy.cc b/chrome/plugin/webplugin_proxy.cc index f41da3e..207b58d 100644 --- a/chrome/plugin/webplugin_proxy.cc +++ b/chrome/plugin/webplugin_proxy.cc @@ -485,6 +485,8 @@ void WebPluginProxy::UpdateGeometry( int ack_key #endif ) { + TransportDIB::ScopedHandle scoped_windowless_handle(windowless_buffer); + TransportDIB::ScopedHandle scoped_background_handle(background_buffer); gfx::Rect old = delegate_->GetRect(); gfx::Rect old_clip_rect = delegate_->GetClipRect(); transparent_ = transparent; @@ -494,7 +496,9 @@ void WebPluginProxy::UpdateGeometry( // synchronous calls that lead to nested UpdateGeometry calls. if (TransportDIB::is_valid(windowless_buffer)) { // The plugin's rect changed, so now we have a new buffer to draw into. - SetWindowlessBuffer(windowless_buffer, background_buffer, window_rect); + SetWindowlessBuffer(scoped_windowless_handle.release(), + scoped_background_handle.release(), + window_rect); } #if defined(OS_MACOSX) @@ -519,61 +523,31 @@ void WebPluginProxy::UpdateGeometry( #endif } -#if defined(OS_WIN) -void WebPluginProxy::SetWindowlessBuffer( - const TransportDIB::Handle& windowless_buffer, - const TransportDIB::Handle& background_buffer, - const gfx::Rect& window_rect) { - // Create a canvas that will reference the shared bits. We have to handle - // errors here since we're mapping a large amount of memory that may not fit - // in our address space, or go wrong in some other way. - windowless_canvas_.reset(new skia::PlatformCanvas); - if (!windowless_canvas_->initialize( - window_rect.width(), - window_rect.height(), - true, - win_util::GetSectionFromProcess(windowless_buffer, - channel_->renderer_handle(), false))) { - windowless_canvas_.reset(); - background_canvas_.reset(); - return; - } - - if (background_buffer) { - background_canvas_.reset(new skia::PlatformCanvas); - if (!background_canvas_->initialize( - window_rect.width(), - window_rect.height(), - true, - win_util::GetSectionFromProcess(background_buffer, - channel_->renderer_handle(), false))) { - windowless_canvas_.reset(); - background_canvas_.reset(); - return; - } - } -} - -#elif defined(OS_MACOSX) - void WebPluginProxy::SetWindowlessBuffer( const TransportDIB::Handle& windowless_buffer, const TransportDIB::Handle& background_buffer, const gfx::Rect& window_rect) { - // Convert the shared memory handle to a handle that works in our process, - // and then use that to create a CGContextRef. + // We have to handle errors here since we're mapping a large amount of memory + // that may not fit in our address space. Also, the renderer may have already + // destroyed the TransportDIB by the time we receive the handle, e.g. in case + // of multiple resizes. +#if defined(OS_MACOSX) windowless_dib_.reset(TransportDIB::Map(windowless_buffer)); background_dib_.reset(TransportDIB::Map(background_buffer)); - windowless_context_.reset(CGBitmapContextCreate( - windowless_dib_->memory(), - window_rect.width(), - window_rect.height(), - 8, 4 * window_rect.width(), - mac_util::GetSystemColorSpace(), - kCGImageAlphaPremultipliedFirst | - kCGBitmapByteOrder32Host)); - CGContextTranslateCTM(windowless_context_, 0, window_rect.height()); - CGContextScaleCTM(windowless_context_, 1, -1); + if (windowless_dib_.get()) { + windowless_context_.reset(CGBitmapContextCreate( + windowless_dib_->memory(), + window_rect.width(), + window_rect.height(), + 8, 4 * window_rect.width(), + mac_util::GetSystemColorSpace(), + kCGImageAlphaPremultipliedFirst | + kCGBitmapByteOrder32Host)); + CGContextTranslateCTM(windowless_context_, 0, window_rect.height()); + CGContextScaleCTM(windowless_context_, 1, -1); + } else { + windowless_context_.reset(); + } if (background_dib_.get()) { background_context_.reset(CGBitmapContextCreate( background_dib_->memory(), @@ -585,32 +559,28 @@ void WebPluginProxy::SetWindowlessBuffer( kCGBitmapByteOrder32Host)); CGContextTranslateCTM(background_context_, 0, window_rect.height()); CGContextScaleCTM(background_context_, 1, -1); + } else { + background_context_.reset(); } -} - -#elif defined(USE_X11) - -void WebPluginProxy::SetWindowlessBuffer( - const TransportDIB::Handle& windowless_buffer, - const TransportDIB::Handle& background_buffer, - const gfx::Rect& window_rect) { +#else int width = window_rect.width(); int height = window_rect.height(); - windowless_dib_.reset(TransportDIB::Map(windowless_buffer)); - if (windowless_dib_.get()) { - windowless_canvas_.reset(windowless_dib_->GetPlatformCanvas(width, height)); - } else { - // This can happen if the renderer has already destroyed the TransportDIB - // by the time we receive the handle, e.g. in case of multiple resizes. + windowless_dib_.reset(TransportDIB::CreateWithHandle(windowless_buffer)); + background_dib_.reset(TransportDIB::CreateWithHandle(background_buffer)); + windowless_canvas_.reset(windowless_dib_->GetPlatformCanvas(width, height)); + background_canvas_.reset(background_dib_->GetPlatformCanvas(width, height)); + if (!windowless_canvas_.get() || !background_canvas_.get()) { windowless_canvas_.reset(); - } - background_dib_.reset(TransportDIB::Map(background_buffer)); - if (background_dib_.get()) { - background_canvas_.reset(background_dib_->GetPlatformCanvas(width, height)); - } else { background_canvas_.reset(); + // Destroy the TransportDIB if the canvas was not created successfully. + // Otherwise we may have an unnecessary handle which is keeping the shared + // memory open. + windowless_dib_.reset(); + background_dib_.reset(); } +#endif +#if defined(USE_X11) // If SHM pixmaps support is available, create a SHM pixmap and // pass it to the delegate for windowless plugin painting. if (delegate_->IsWindowless() && use_shm_pixmap_ && windowless_dib_.get()) { @@ -630,9 +600,8 @@ void WebPluginProxy::SetWindowlessBuffer( delegate_->SetWindowlessShmPixmap(windowless_shm_pixmap_); } -} - #endif +} void WebPluginProxy::CancelDocumentLoad() { Send(new PluginHostMsg_CancelDocumentLoad(route_id_)); diff --git a/chrome/plugin/webplugin_proxy.h b/chrome/plugin/webplugin_proxy.h index a33dae7..667bb63 100644 --- a/chrome/plugin/webplugin_proxy.h +++ b/chrome/plugin/webplugin_proxy.h @@ -205,9 +205,9 @@ class WebPluginProxy : public webkit_glue::WebPlugin { // Variables used for desynchronized windowless plugin painting. See note in // webplugin_delegate_proxy.h for how this works. bool transparent_; -#if defined(OS_MACOSX) scoped_ptr<TransportDIB> windowless_dib_; scoped_ptr<TransportDIB> background_dib_; +#if defined(OS_MACOSX) base::mac::ScopedCFTypeRef<CGContextRef> windowless_context_; base::mac::ScopedCFTypeRef<CGContextRef> background_context_; scoped_ptr<WebPluginAcceleratedSurfaceProxy> accelerated_surface_; @@ -216,8 +216,6 @@ class WebPluginProxy : public webkit_glue::WebPlugin { scoped_ptr<skia::PlatformCanvas> background_canvas_; #if defined(USE_X11) - scoped_ptr<TransportDIB> windowless_dib_; - scoped_ptr<TransportDIB> background_dib_; // If we can use SHM pixmaps for windowless plugin painting or not. bool use_shm_pixmap_; // The SHM pixmap for windowless plugin painting. diff --git a/chrome/renderer/media/ipc_video_renderer.cc b/chrome/renderer/media/ipc_video_renderer.cc index d9a194d..15d05e6 100644 --- a/chrome/renderer/media/ipc_video_renderer.cc +++ b/chrome/renderer/media/ipc_video_renderer.cc @@ -160,6 +160,7 @@ void IPCVideoRenderer::DoUpdateVideo() { Send(new ViewHostMsg_UpdateVideo(routing_id_, transport_dib_->id(), + transport_dib_->handle(), video_rect_)); } diff --git a/chrome/renderer/nacl_desc_wrapper_chrome.cc b/chrome/renderer/nacl_desc_wrapper_chrome.cc index a4efc70..45a5a36 100644 --- a/chrome/renderer/nacl_desc_wrapper_chrome.cc +++ b/chrome/renderer/nacl_desc_wrapper_chrome.cc @@ -33,7 +33,7 @@ DescWrapper* DescWrapperFactory::ImportPepper2DSharedMemory(intptr_t shm_int) { return ImportShmHandle(dib->handle().fd, dib->size()); #elif defined(OS_WIN) // TransportDIBs use MapViewOfFile shared memory on Windows. - return ImportShmHandle(dib->handle(), dib->size()); + return ImportShmHandle(dib->handle().section(), dib->size()); #else # error "What platform?" #endif diff --git a/chrome/renderer/render_widget.cc b/chrome/renderer/render_widget.cc index 8880a03..2e7d975 100644 --- a/chrome/renderer/render_widget.cc +++ b/chrome/renderer/render_widget.cc @@ -508,6 +508,7 @@ void RenderWidget::DoDeferredUpdate() { // A plugin may be able to do an optimized paint. First check this, in which // case we can skip all of the bitmap generation and regular paint code. TransportDIB::Id dib_id = TransportDIB::Id(); + TransportDIB::Handle dib_handle = TransportDIB::DefaultHandleValue(); TransportDIB* dib = NULL; std::vector<gfx::Rect> copy_rects; gfx::Rect optimized_copy_rect, optimized_copy_location; @@ -518,6 +519,7 @@ void RenderWidget::DoDeferredUpdate() { bounds = optimized_copy_location; copy_rects.push_back(optimized_copy_rect); dib_id = dib->id(); + dib_handle = dib->handle(); } else if (!is_gpu_rendering_active_) { // Compute a buffer for painting and cache it. scoped_ptr<skia::PlatformCanvas> canvas( @@ -553,6 +555,7 @@ void RenderWidget::DoDeferredUpdate() { PaintRect(copy_rects[i], bounds.origin(), canvas.get()); dib_id = current_paint_buf_->id(); + dib_handle = current_paint_buf_->handle(); } else { // Accelerated compositing path // Begin painting. bool finish = next_paint_is_resize_ack(); @@ -561,7 +564,8 @@ void RenderWidget::DoDeferredUpdate() { // sending an ack to browser process that the paint is complete... ViewHostMsg_UpdateRect_Params params; - params.bitmap = dib_id; + params.dib_id = dib_id; + params.dib_handle = dib_handle; params.bitmap_rect = bounds; params.dx = update.scroll_delta.x(); params.dy = update.scroll_delta.y(); @@ -819,7 +823,8 @@ void RenderWidget::OnMsgPaintAtSize(const TransportDIB::Handle& dib_handle, int tag, const gfx::Size& page_size, const gfx::Size& desired_size) { - if (!webwidget_ || dib_handle == TransportDIB::DefaultHandleValue()) + TransportDIB::ScopedHandle scoped_dib_handle(dib_handle); + if (!webwidget_ || !TransportDIB::is_valid(dib_handle)) return; if (page_size.IsEmpty() || desired_size.IsEmpty()) { @@ -831,13 +836,17 @@ void RenderWidget::OnMsgPaintAtSize(const TransportDIB::Handle& dib_handle, // Map the given DIB ID into this process, and unmap it at the end // of this function. - scoped_ptr<TransportDIB> paint_at_size_buffer(TransportDIB::Map(dib_handle)); - - DCHECK(paint_at_size_buffer.get()); - if (!paint_at_size_buffer.get()) + scoped_ptr<TransportDIB> paint_at_size_buffer( + TransportDIB::CreateWithHandle(scoped_dib_handle.release())); + gfx::Size canvas_size = page_size; + scoped_ptr<skia::PlatformCanvas> canvas( + paint_at_size_buffer->GetPlatformCanvas(canvas_size.width(), + canvas_size.height())); + if (!canvas.get()) { + NOTREACHED(); return; + } - gfx::Size canvas_size = page_size; float x_scale = static_cast<float>(desired_size.width()) / static_cast<float>(canvas_size.width()); float y_scale = static_cast<float>(desired_size.height()) / @@ -848,14 +857,6 @@ void RenderWidget::OnMsgPaintAtSize(const TransportDIB::Handle& dib_handle, canvas_size.set_height(static_cast<int>(canvas_size.height() * y_scale)); gfx::Rect bounds(canvas_size); - scoped_ptr<skia::PlatformCanvas> canvas( - paint_at_size_buffer->GetPlatformCanvas(canvas_size.width(), - canvas_size.height())); - if (!canvas.get()) { - NOTREACHED(); - return; - } - // Reset bounds to what we actually received, but they should be the // same. DCHECK_EQ(bounds.width(), canvas->getDevice()->width()); diff --git a/chrome/renderer/webplugin_delegate_proxy.cc b/chrome/renderer/webplugin_delegate_proxy.cc index a2553f2..7b3938e 100644 --- a/chrome/renderer/webplugin_delegate_proxy.cc +++ b/chrome/renderer/webplugin_delegate_proxy.cc @@ -1506,9 +1506,10 @@ void WebPluginDelegateProxy::OnAcceleratedSurfaceSetTransportDIB( int32 width, int32 height, TransportDIB::Handle transport_dib) { + TransportDIB::ScopedHandle scoped_dib_handle(transport_dib); if (render_view_) render_view_->AcceleratedSurfaceSetTransportDIB(window, width, height, - transport_dib); + scoped_dib_handle.release()); } void WebPluginDelegateProxy::OnAcceleratedSurfaceAllocTransportDIB( |