[BlobAsync] Fixed shared memory mapping

content/child/blob_storage/blob_transport_controller.cc

 // Copyright 2015 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 "content/child/blob_storage/blob_transport_controller.h"
 
 #include <limits>
 #include <memory>
 #include <utility>
 #include <vector>
@@ skipping 193 matching lines @@
   BlobConsolidation* consolidation = it->second.get();
   const auto& consolidated_items = consolidation->consolidated_items();
 
   std::unique_ptr<std::vector<BlobItemBytesRequest>> file_requests(
       new std::vector<BlobItemBytesRequest>());
 
   // Since we can be writing to the same shared memory handle from multiple
   // requests, we keep them in a vector and lazily create them.
   ScopedVector<SharedMemory> opened_memory;
   opened_memory.resize(memory_handles->size());
+
+  // We need to calculate how much memory we expect to be writing to the memory
+  // segments so we can correctly map it the first time.
+  std::vector<size_t> shared_memory_sizes;

[Marijn Kruisselbrink, 2016/06/21 16:52:35]

nit: Why not just pass memory_handles->size() directly to the vector
constructor, rather than the split between constructing and resizing?

[dmurph, 2016/06/21 17:33:03]

Ah, right, thanks.

+  shared_memory_sizes.resize(memory_handles->size(), 0);
+  for (const BlobItemBytesRequest& request : requests) {
+    if (request.transport_strategy !=
+        IPCBlobItemRequestStrategy::SHARED_MEMORY) {
+      continue;
+    }
+    shared_memory_sizes[request.handle_index] =
+        std::max(shared_memory_sizes[request.handle_index],
+                 static_cast<size_t>(request.size + request.handle_offset));

[Marijn Kruisselbrink, 2016/06/21 16:52:35]

rather than the static_cast<size_t> you could also just explicitly pass the
template parameter to std::max (so std::max<size_t>(..., ...))

[dmurph, 2016/06/21 17:33:03]

Done.

+  }
+
   for (const BlobItemBytesRequest& request : requests) {
     DCHECK_LT(request.renderer_item_index, consolidated_items.size())
         << "Invalid item index";
 
     const ConsolidatedItem& item =
         consolidated_items[request.renderer_item_index];
     DCHECK_LE(request.renderer_item_offset + request.size, item.length)
         << "Invalid data range";
     DCHECK_EQ(item.type, DataElement::TYPE_BYTES) << "Invalid element type";
 
     switch (request.transport_strategy) {
       case IPCBlobItemRequestStrategy::IPC: {
         responses.push_back(BlobItemBytesResponse(request.request_number));
         BlobItemBytesResponse& response = responses.back();
         ReadStatus status = consolidation->ReadMemory(
             request.renderer_item_index, request.renderer_item_offset,
             request.size, response.allocate_mutable_data(request.size));
         DCHECK(status == ReadStatus::OK)
             << "Error reading from consolidated blob: "
             << static_cast<int>(status);
         break;
       }
       case IPCBlobItemRequestStrategy::SHARED_MEMORY: {
         responses.push_back(BlobItemBytesResponse(request.request_number));
         DCHECK_LT(request.handle_index, memory_handles->size())

[Marijn Kruisselbrink, 2016/06/21 16:52:35]

Should you move/copy this DCHECK to your new loop?

[dmurph, 2016/06/21 17:33:03]

Done.

             << "Invalid handle index.";
         SharedMemory* memory = opened_memory[request.handle_index];
         if (!memory) {
           SharedMemoryHandle& handle = (*memory_handles)[request.handle_index];
+          size_t size = shared_memory_sizes[request.handle_index];
           DCHECK(SharedMemory::IsHandleValid(handle));
           std::unique_ptr<SharedMemory> shared_memory(
               new SharedMemory(handle, false));
 
-          if (!shared_memory->Map(request.size)) {
+          if (!shared_memory->Map(size)) {
             // This would happen if the renderer process doesn't have enough
             // memory to map the shared memory, which is possible if we don't
             // have much memory. If this scenario happens often, we could delay
             // the response until we have enough memory.  For now we just fail.
             CHECK(false) << "Unable to map shared memory to send blob " << uuid
                          << ".";
             return;
           }
           memory = shared_memory.get();
           opened_memory[request.handle_index] = shared_memory.release();
@@ skipping 139 matching lines @@
 
 void BlobTransportController::ReleaseBlobConsolidation(
     const std::string& uuid) {
   if (blob_storage_.erase(uuid)) {
     main_thread_runner_->PostTask(FROM_HERE,
                                   base::Bind(&DecChildProcessRefCount));
   }
 }
 
 }  // namespace content