Make SharedMemory track the size that was actually mapped

ui/surface/transport_dib_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/surface/transport_dib.h"
 
 #include <windows.h>
 
 #include <limits>
 
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/sys_info.h"
 #include "skia/ext/platform_canvas.h"
 
-TransportDIB::TransportDIB() {
+TransportDIB::TransportDIB()
+    : size_(0) {
 }
 
 TransportDIB::~TransportDIB() {
 }
 
 TransportDIB::TransportDIB(HANDLE handle)
-    : shared_memory_(handle, false /* read write */) {
+    : shared_memory_(handle, false /* read write */),
+      size_(0) {
 }
 
 // static
 TransportDIB* TransportDIB::Create(size_t size, uint32 sequence_num) {
-  size_t allocation_granularity = base::SysInfo::VMAllocationGranularity();
-  size = size / allocation_granularity + 1;
-  size = size * allocation_granularity;
-
   TransportDIB* dib = new TransportDIB;
 
   if (!dib->shared_memory_.CreateAnonymous(size)) {
     delete dib;
     return NULL;
   }
 
   dib->size_ = size;
   dib->sequence_num_ = sequence_num;
 
@@ skipping 21 matching lines @@
 // static
 bool TransportDIB::is_valid_id(TransportDIB::Id id) {
   return is_valid_handle(id.handle);
 }
 
 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.";
 
-  return skia::CreatePlatformCanvas(w, h, true, handle(),
-                                    skia::RETURN_NULL_ON_FAILURE);
+  // We can't check the canvas size before mapping, but it's safe because
+  // Windows will fail to map the section if the dimensions of the canvas
+  // are too large.
+  skia::PlatformCanvas* canvas =
+      skia::CreatePlatformCanvas(w, h, true, handle(),
+                                 skia::RETURN_NULL_ON_FAILURE);
+
+  // Get the size for the memory region backing the canvas.
+  if (canvas) {
+    MEMORY_BASIC_INFORMATION memory_info;
+    if (!::VirtualQuery(canvas, &memory_info, sizeof(memory_info))) {
+      delete canvas;
+      return NULL;
+    }
+
+    size_ = memory_info.RegionSize - (reinterpret_cast<char*>(canvas) -

[cpu_(ooo_6.6-7.5), 2013/03/21 17:33:40]

do we have any guarantee that skia::CreatePlatformCanvas returns a pointer
something inside the section?

I mean could skia::PlatformCanvas be

class PlatformCanvas {
  int foo_;
  void* section_;
};

[jschuh, 2013/03/21 19:32:06]

As written now, yes. It just ends up calling CreateDIBSection, which maps the
section as the canvas. I'm not sure what we could do to future-proof this,
beyond what I'm already doing by checking with the VirtualQuery after the
mapping. And someone was opposed to me using native API calls to perform any
more intensive verification. ;)

[cpu_(ooo_6.6-7.5), 2013/03/21 20:02:27]

But what does the subtraction of line 86 and 87 do then? 

Seems we need a dcheck in the skia::CreatePlatformCanvas function.


On 2013/03/21 19:32:06, Justin Schuh wrote:

[jschuh, 2013/03/26 22:09:24]

I think I over-complicated things. We can just calculate the size here since we
know that either the canvas is valid or creating it failed.

+           static_cast<char*>(memory_info.AllocationBase));
+  }
+
+  return canvas;
 }
 
 bool TransportDIB::Map() {
   if (!is_valid_handle(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();
+  size_ = shared_memory_.mapped_size();
   return true;
 }
 
 void* TransportDIB::memory() const {
   return shared_memory_.memory();
 }
 
 TransportDIB::Handle TransportDIB::handle() const {
   return shared_memory_.handle();
 }
 
 TransportDIB::Id TransportDIB::id() const {
   return Id(handle(), sequence_num_);
 }