[iOS] Fixed tab snapshot cache refresh issues on iPad.

ios/chrome/browser/snapshots/snapshot_cache.mm

 // Copyright 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.
 
 #import "ios/chrome/browser/snapshots/snapshot_cache.h"
 
 #import <UIKit/UIKit.h>
 
 #include "base/critical_closure.h"
-#include "base/mac/bind_objc_block.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/mac/bind_objc_block.h"
 #include "base/mac/scoped_cftyperef.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/task_runner_util.h"
 #include "base/threading/thread_restrictions.h"
@@ skipping 116 matching lines @@
 }
 
 - (id)init {
   if ((self = [super init])) {
     DCHECK_CURRENTLY_ON_WEB_THREAD(web::WebThread::UI);
     propertyReleaser_SnapshotCache_.Init(self, [SnapshotCache class]);
 
     // TODO(andybons): In the case where the cache grows, it is expensive.
     // Make sure this doesn't suck when there are more than ten tabs.
     imageDictionary_.reset(
         [[NSMutableDictionary alloc] initWithCapacity:kCacheInitialCapacity]);

[noyau (Ping after 24h), 2015/05/19 21:36:35]

No need to allocate this on tablets.

[sdefresne, 2015/05/20 13:15:12]

Done.

     [[NSNotificationCenter defaultCenter]
         addObserver:self
            selector:@selector(handleLowMemory)
                name:UIApplicationDidReceiveMemoryWarningNotification
              object:nil];
     [[NSNotificationCenter defaultCenter]
         addObserver:self
            selector:@selector(handleEnterBackground)
                name:UIApplicationDidEnterBackgroundNotification
              object:nil];
@@ skipping 41 matching lines @@
   UIImage* img = [imageDictionary_ objectForKey:sessionID];
   if (img) {
     if (callback)
       callback(img);
     return;
   }
 
   base::PostTaskAndReplyWithResult(
       web::WebThread::GetMessageLoopProxyForThread(
           web::WebThread::FILE_USER_BLOCKING).get(),
-      FROM_HERE,
-      base::BindBlock(^base::scoped_nsobject<UIImage>() {
+      FROM_HERE, base::BindBlock(^base::scoped_nsobject<UIImage>() {
         // Retrieve the image on a high priority thread.
         return base::scoped_nsobject<UIImage>([ReadImageFromDisk(
             [SnapshotCache imagePathForSessionID:sessionID]) retain]);
       }),
       base::BindBlock(^(base::scoped_nsobject<UIImage> image) {
-        if (image)
+        // The iPad tab switcher is currently using its own memory cache so the
+        // image is not stored in memory here if running on iPad.
+        // The same logic is used on image writes (code below).
+        if (!IsIPadIdiom() && image)
           [imageDictionary_ setObject:image forKey:sessionID];
         if (callback)
           callback(image);

[noyau (Ping after 24h), 2015/05/19 21:36:34]

Why would we retrieve the image if the callback is nil? On phone, it may be used
to prefill the cache, but on iPad this is a very bad idea. At least in this case
don't call the PostTask. 

Ideally can you add a DCHECK(!IsIPadIdiom()||callback) to the method (making
sure nothing calls it that way)?

[sdefresne, 2015/05/20 13:15:12]

Changed the method to be a no-op on iPad if there is no callback. I didn't use
DCHECK(!IsIPadIdiom()) as that may break downstream as the method is part of
public API.

       }));
 }
 
 - (void)setImage:(UIImage*)img withSessionID:(NSString*)sessionID {
   DCHECK_CURRENTLY_ON_WEB_THREAD(web::WebThread::UI);
   if (!img || !sessionID)
     return;
 
-  // Color snapshots are not used on tablets, so don't keep them in memory.
+  // The iPad tab switcher is currently using its own memory cache so the image
+  // is not stored in memory here if running on iPad.
+  // The same logic is used on image reads (code above).
   if (!IsIPadIdiom()) {
     [imageDictionary_ setObject:img forKey:sessionID];
   }
   // Save the image to disk.
   web::WebThread::PostBlockingPoolSequencedTask(
       kSequenceToken, FROM_HERE,
       base::BindBlock(^{
         base::scoped_nsobject<UIImage> image([img retain]);
         WriteImageToDisk(image,
                          [SnapshotCache imagePathForSessionID:sessionID]);
@@ skipping 109 matching lines @@
 
 - (void)handleLowMemory {
   DCHECK_CURRENTLY_ON_WEB_THREAD(web::WebThread::UI);
   NSMutableDictionary* dictionary =
       [[NSMutableDictionary alloc] initWithCapacity:2];
   for (NSString* sessionID in pinnedIDs_) {
     UIImage* image = [imageDictionary_ objectForKey:sessionID];
     if (image)
       [dictionary setObject:image forKey:sessionID];
   }
   imageDictionary_.reset(dictionary);

[noyau (Ping after 24h), 2015/05/19 21:36:35]

Same here, no need to even run anything here on ipad.

[sdefresne, 2015/05/20 13:15:12]

Done.

 }
 
 - (void)handleEnterBackground {
   DCHECK_CURRENTLY_ON_WEB_THREAD(web::WebThread::UI);
   [imageDictionary_ removeAllObjects];
 }
 
 - (void)handleBecomeActive {
   DCHECK_CURRENTLY_ON_WEB_THREAD(web::WebThread::UI);
   for (NSString* sessionID in pinnedIDs_)
     [self retrieveImageForSessionID:sessionID callback:nil];

[noyau (Ping after 24h), 2015/05/19 21:36:35]

Not on ipad…

[sdefresne, 2015/05/20 13:15:12]

Done.

 }
 
 - (void)saveGreyImage:(UIImage*)greyImage forKey:(NSString*)sessionID {
   DCHECK_CURRENTLY_ON_WEB_THREAD(web::WebThread::UI);
   if (greyImage)
     [greyImageDictionary_ setObject:greyImage forKey:sessionID];
   if ([sessionID isEqualToString:mostRecentGreySessionId_]) {
     mostRecentGreyBlock_.get()(greyImage);
     [self clearGreySessionInfo];
   }
@@ skipping 120 matching lines @@
       backgroundingImageSessionId_.reset();
     }
   }
 
   web::WebThread::PostBlockingPoolTask(
       FROM_HERE, base::Bind(&ConvertAndSaveGreyImage, colorImagePath,
                             greyImagePath, backgroundingColorImage_));
 }
 
 @end