Make user_manager::UserImage non-copyable

components/wallpaper/wallpaper_manager_base.cc

 // Copyright 2014 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 "components/wallpaper/wallpaper_manager_base.h"
 
 #include <numeric>
 #include <vector>
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 892 matching lines @@
       delay = base::TimeDelta::FromMilliseconds(0);
     else if (interval > base::TimeDelta::FromMilliseconds(0))
       delay -= interval;
   }
   return delay;
 }
 
 void WallpaperManagerBase::OnCustomizedDefaultWallpaperDecoded(
     const GURL& wallpaper_url,
     scoped_ptr<CustomizedWallpaperRescaledFiles> rescaled_files,
-    const user_manager::UserImage& wallpaper) {
+    scoped_ptr< user_manager::UserImage> wallpaper) {

[hashimoto, 2016/03/15 08:39:27]

nit: "< " -> "<"

[satorux1, 2016/03/16 02:01:30]

Done.

   DCHECK(thread_checker_.CalledOnValidThread());
 
   // If decoded wallpaper is empty, we have probably failed to decode the file.
-  if (wallpaper.image().isNull()) {
+  if (wallpaper->image().isNull()) {
     LOG(WARNING) << "Failed to decode customized wallpaper.";
     return;
   }
 
-  wallpaper.image().EnsureRepsForSupportedScales();
-  scoped_ptr<gfx::ImageSkia> deep_copy(wallpaper.image().DeepCopy());
+  wallpaper->image().EnsureRepsForSupportedScales();
+  scoped_ptr<gfx::ImageSkia> deep_copy(wallpaper->image().DeepCopy());

[hashimoto, 2016/03/15 08:39:27]

Do we still need to deep copy the image here?
Can't we just modify the passed image which we have ownership now?

[satorux1, 2016/03/16 02:01:30]

Good point. Wanted to test but this code path is hard to test. If I understand
correctly, this code is used to handle vendor-specified wallpapers downloaded
from the web via CustomizationDocument:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...


I'll spend a bit more time to figure how to test the code, but for now let me
add a TODO.

 
   scoped_ptr<bool> success(new bool(false));
   scoped_ptr<gfx::ImageSkia> small_wallpaper_image(new gfx::ImageSkia);
   scoped_ptr<gfx::ImageSkia> large_wallpaper_image(new gfx::ImageSkia);
 
   // TODO(bshe): This may break if Bytes becomes RefCountedMemory.
   base::Closure resize_closure = base::Bind(
       &WallpaperManagerBase::ResizeCustomizedDefaultWallpaper,
-      base::Passed(&deep_copy), wallpaper.image_bytes(),
+      base::Passed(&deep_copy), wallpaper->image_bytes(),

[hashimoto, 2016/03/15 08:39:27]

This line still copies image bytes.

[satorux1, 2016/03/16 02:01:30]

Added a TODO comment.

       base::Unretained(rescaled_files.get()), base::Unretained(success.get()),
       base::Unretained(small_wallpaper_image.get()),
       base::Unretained(large_wallpaper_image.get()));
   base::Closure on_resized_closure = base::Bind(
       &WallpaperManagerBase::OnCustomizedDefaultWallpaperResized,
       weak_factory_.GetWeakPtr(), wallpaper_url,
       base::Passed(std::move(rescaled_files)), base::Passed(std::move(success)),
       base::Passed(std::move(small_wallpaper_image)),
       base::Passed(std::move(large_wallpaper_image)));
 
@@ skipping 63 matching lines @@
 void WallpaperManagerBase::CreateSolidDefaultWallpaper() {
   loaded_wallpapers_for_test_++;
   SkBitmap bitmap;
   bitmap.allocN32Pixels(1, 1);
   bitmap.eraseColor(kDefaultWallpaperColor);
   const gfx::ImageSkia image = gfx::ImageSkia::CreateFrom1xBitmap(bitmap);
   default_wallpaper_image_.reset(new user_manager::UserImage(image));
 }
 
 }  // namespace wallpaper