Fix ThumbnailGenerator on Windows.

In the browser, duplicate the file mapping handle before sending to the
renderer and map the TransportDIB before accessing the memory. In the
renderer, do not map the file twice, which was happening when
TransportDIB::Map and GetPlatformCanvas were both being called.
BUG=none
TEST=none

Review URL: http://codereview.chromium.org/4569002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@65775 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 123 insertions, 44 deletions

diff --git a/app/surface/transport_dib.h b/app/surface/transport_dib.h
index 6606c2b..d82c346 100644
--- a/app/surface/transport_dib.h
+++ b/app/surface/transport_dib.h
@@ -124,6 +124,10 @@ class TransportDIB {
   // 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 +135,19 @@ 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 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..9976e50 100644
--- a/app/surface/transport_dib_linux.cc
+++ b/app/surface/transport_dib_linux.cc
@@ -65,34 +65,54 @@ 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 (!is_valid(key_))
+    return false;
+  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;
+}
+
 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 c3f3abd2..8ea81ae 100644
--- a/app/surface/transport_dib_mac.cc
+++ b/app/surface/transport_dib_mac.cc
@@ -38,36 +38,48 @@ TransportDIB* TransportDIB::Create(size_t size, uint32 sequence_num) {
 }
 
 // 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 (!is_valid(handle()))
+    return false;
+  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;
+}
+
 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 63506b2..22812c9 100644
--- a/app/surface/transport_dib_win.cc
+++ b/app/surface/transport_dib_win.cc
@@ -2,10 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <limits>
+#include "app/surface/transport_dib.h"
+
 #include <windows.h>
 
-#include "app/surface/transport_dib.h"
+#include <limits>
+
 #include "base/logging.h"
 #include "base/scoped_ptr.h"
 #include "base/sys_info.h"
@@ -41,36 +43,55 @@ 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) {
+  return new TransportDIB(handle);
 }
 
+// static
 bool TransportDIB::is_valid(Handle dib) {
   return dib != NULL;
 }
 
 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()))
     return NULL;
   return canvas.release();
 }
 
+bool TransportDIB::Map() {
+  if (!is_valid(handle()))
+    return false;
+  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;
+}
+
 void* TransportDIB::memory() const {
   return shared_memory_.memory();
 }
@@ -80,5 +101,5 @@ TransportDIB::Handle TransportDIB::handle() const {
 }
 
 TransportDIB::Id TransportDIB::id() const {
-  return Id(shared_memory_.handle(), sequence_num_);
+  return Id(handle(), sequence_num_);
 }
diff --git a/app/win_util.cc b/app/win_util.cc
index 5232e34..be267af 100644
--- a/app/win_util.cc
+++ b/app/win_util.cc
@@ -269,6 +269,16 @@ HANDLE GetSectionFromProcess(HANDLE section, HANDLE process, bool read_only) {
   return valid_section;
 }
 
+HANDLE GetSectionForProcess(HANDLE section, HANDLE process, bool read_only) {
+  HANDLE valid_section = NULL;
+  DWORD access = STANDARD_RIGHTS_REQUIRED | FILE_MAP_READ;
+  if (!read_only)
+    access |= FILE_MAP_WRITE;
+  DuplicateHandle(GetCurrentProcess(), section, process, &valid_section, access,
+                  FALSE, 0);
+  return valid_section;
+}
+
 bool DoesWindowBelongToActiveWindow(HWND window) {
   DCHECK(window);
   HWND top_window = ::GetAncestor(window, GA_ROOT);
diff --git a/app/win_util.h b/app/win_util.h
index 34c2586..ab5cbc3 100644
--- a/app/win_util.h
+++ b/app/win_util.h
@@ -148,6 +148,10 @@ bool EdgeHasTopmostAutoHideTaskbar(UINT edge, HMONITOR monitor);
 // Returns the new valid handle if the function succeed. NULL otherwise.
 HANDLE GetSectionFromProcess(HANDLE section, HANDLE process, bool read_only);
 
+// Duplicates a section handle from the current process for use in another
+// process. Returns the new valid handle or NULL on failure.
+HANDLE GetSectionForProcess(HANDLE section, HANDLE process, bool read_only);
+
 // Returns true if the specified window is the current active top window or one
 // of its children.
 bool DoesWindowBelongToActiveWindow(HWND window);