Don't overwrit image data case entries that are just marked usable.

ppapi/proxy/ppb_image_data_proxy.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 "ppapi/proxy/ppb_image_data_proxy.h"
 
 #include <string.h>  // For memcpy
 
 #include <map>
 #include <vector>
@@ skipping 112 matching lines @@
   scoped_refptr<ImageData> Get(int width, int height,
                                PP_ImageDataFormat format);
   void Add(ImageData* image_data);
   void ImageDataUsable(ImageData* image_data);
 
   // Expires old entries. Returns true if there are still entries in the list,
   // false if this instance cache is now empty.
   bool ExpireEntries();
 
  private:
+  void IncrementInsertionPoint();
+
   // We'll store this many ImageDatas per instance.
   const static int kCacheSize = 2;
 
   ImageDataCacheEntry images_[kCacheSize];
 
   // Index into cache where the next item will go.
   int next_insertion_point_;
 };
 
 scoped_refptr<ImageData> ImageDataInstanceCache::Get(
     int width, int height,
     PP_ImageDataFormat format) {
   // Just do a brute-force search since the cache is so small.
   for (int i = 0; i < kCacheSize; i++) {
     if (!images_[i].usable)
       continue;
     const PP_ImageDataDesc& desc = images_[i].image->desc();
     if (desc.format == format &&
         desc.size.width == width && desc.size.height == height) {
       scoped_refptr<ImageData> ret(images_[i].image);
       images_[i] = ImageDataCacheEntry();
+
+      // Since we just removed an item, this entry is the best place to insert
+      // a subsequent one.
+      next_insertion_point_ = i;
       return ret;
     }
   }
   return scoped_refptr<ImageData>();
 }
 
 void ImageDataInstanceCache::Add(ImageData* image_data) {
   images_[next_insertion_point_] = ImageDataCacheEntry(image_data);
-
-  // Go to the next location, wrapping around to get LRU.
-  next_insertion_point_++;
-  if (next_insertion_point_ >= kCacheSize)
-    next_insertion_point_ = 0;
+  IncrementInsertionPoint();
 }
 
 void ImageDataInstanceCache::ImageDataUsable(ImageData* image_data) {
   for (int i = 0; i < kCacheSize; i++) {
     if (images_[i].image.get() == image_data) {
       images_[i].usable = true;
+
+      // This test is important. The renderer doesn't guarantee how many image
+      // datas it has or when it notifies us when one is usable. Its possible
+      // to get into situations where it's always telling us the old one is
+      // usable, and then the older one immediately gets expired. Therefore,
+      // if the next insertion would overwrite this now-usable entry, make the
+      // next insertion overwrite some other entry to avoid the replacement.
+      if (next_insertion_point_ == i)
+        IncrementInsertionPoint();
       return;
     }
   }
 }
 
 bool ImageDataInstanceCache::ExpireEntries() {
   base::TimeTicks threshold_time =
       base::TimeTicks::Now() - base::TimeDelta::FromSeconds(kMaxAgeSeconds);
 
   bool has_entry = false;
   for (int i = 0; i < kCacheSize; i++) {
     if (images_[i].image.get()) {
       // Entry present.
       if (images_[i].added_time <= threshold_time) {
         // Found an entry to expire.
         images_[i] = ImageDataCacheEntry();
         next_insertion_point_ = i;
       } else {
         // Found an entry that we're keeping.
         has_entry = true;
       }
     }
   }
   return has_entry;
 }
 
+void ImageDataInstanceCache::IncrementInsertionPoint() {
+  // Go to the next location, wrapping around to get LRU.
+  next_insertion_point_++;
+  if (next_insertion_point_ >= kCacheSize)
+    next_insertion_point_ = 0;
+}
+
 // ImageDataCache --------------------------------------------------------------
 
 class ImageDataCache {
  public:
   ImageDataCache() : weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {}
   ~ImageDataCache() {}
 
   static ImageDataCache* GetInstance();
 
   // Retrieves an image data from the cache of the specified size and format if
@@ skipping 72 matching lines @@
 
 }  // namespace
 
 // ImageData -------------------------------------------------------------------
 
 #if !defined(OS_NACL)
 ImageData::ImageData(const HostResource& resource,
                      const PP_ImageDataDesc& desc,
                      ImageHandle handle)
     : Resource(OBJECT_IS_PROXY, resource),
-      desc_(desc) {
+      desc_(desc),
+      used_in_replace_contents_(false) {
 #if defined(OS_WIN)
   transport_dib_.reset(TransportDIB::CreateWithHandle(handle));
 #else
   transport_dib_.reset(TransportDIB::Map(handle));
 #endif  // defined(OS_WIN)
 }
 #else  // !defined(OS_NACL)
 
 ImageData::ImageData(const HostResource& resource,
                      const PP_ImageDataDesc& desc,
                      const base::SharedMemoryHandle& handle)
     : Resource(OBJECT_IS_PROXY, resource),
       desc_(desc),
       shm_(handle, false /* read_only */),
       size_(desc.size.width * desc.size.height * 4),
-      map_count_(0) {
+      map_count_(0),
+      used_in_replace_contents_(false) {
 }
 #endif  // !defined(OS_NACL)
 
 ImageData::~ImageData() {
 }
 
 PPB_ImageData_API* ImageData::AsPPB_ImageData_API() {
   return this;
 }
 
 void ImageData::LastPluginRefWasDeleted() {
-  // The plugin no longer needs this ImageData, add it to our cache.
-  ImageDataCache::GetInstance()->Add(this);
+  // The plugin no longer needs this ImageData, add it to our cache if it's
+  // been used in a ReplaceContents. These are the ImageDatas that the renderer
+  // will send back ImageDataUsable messages for.
+  if (used_in_replace_contents_)
+    ImageDataCache::GetInstance()->Add(this);
 }
 
 PP_Bool ImageData::Describe(PP_ImageDataDesc* desc) {
   memcpy(desc, &desc_, sizeof(PP_ImageDataDesc));
   return PP_TRUE;
 }
 
 void* ImageData::Map() {
 #if defined(OS_NACL)
   if (map_count_++ == 0)
@@ skipping 41 matching lines @@
 }
 
 SkCanvas* ImageData::GetCanvas() {
 #if defined(OS_NACL)
   return NULL;  // No canvas in NaCl.
 #else
   return mapped_canvas_.get();
 #endif
 }
 
-void ImageData::ZeroContents() {
-  void* data = Map();
-  memset(data, 0, desc_.stride * desc_.size.height);
-  Unmap();
+void ImageData::RecycleToPlugin(bool zero_contents) {
+  used_in_replace_contents_ = false;
+  if (zero_contents) {
+    void* data = Map();
+    memset(data, 0, desc_.stride * desc_.size.height);
+    Unmap();
+  }
 }
 
 #if !defined(OS_NACL)
 // static
 ImageHandle ImageData::NullHandle() {
 #if defined(OS_WIN)
   return NULL;
 #elif defined(OS_MACOSX) || defined(OS_ANDROID)
   return ImageHandle();
 #else
@@ skipping 28 matching lines @@
                                                      PP_Bool init_to_zero) {
   PluginDispatcher* dispatcher = PluginDispatcher::GetForInstance(instance);
   if (!dispatcher)
     return 0;
 
   // Check the cache.
   scoped_refptr<ImageData> cached_image_data =
       ImageDataCache::GetInstance()->Get(instance, size.width, size.height,
                                          format);
   if (cached_image_data.get()) {
-    // We have one we can re-use rather than allocating a new one. Only zero
-    // out if requested.
-    if (PP_ToBool(init_to_zero))
-      cached_image_data->ZeroContents();
+    // We have one we can re-use rather than allocating a new one.
+    cached_image_data->RecycleToPlugin(PP_ToBool(init_to_zero));
     return cached_image_data->GetReference();
   }
 
   HostResource result;
   std::string image_data_desc;
 #if defined(OS_NACL)
   base::SharedMemoryHandle image_handle = base::SharedMemory::NULLHandle();
   dispatcher->Send(new PpapiHostMsg_PPBImageData_CreateNaCl(
       kApiID, instance, format, size, init_to_zero,
       &result, &image_data_desc, &image_handle));
@@ skipping 149 matching lines @@
   // still cached in our process, the proxy still holds a reference so we can
   // remove the one the renderer just sent is. If the proxy no longer holds a
   // reference, we released everything and we should also release the one the
   // renderer just sent us.
   dispatcher()->Send(new PpapiHostMsg_PPBCore_ReleaseResource(
       API_ID_PPB_CORE, old_image_data));
 }
 
 }  // namespace proxy
 }  // namespace ppapi