Use factory for creating SkPicture from a stream.

cc/resources/picture.cc

 // 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.
 
 #include "cc/resources/picture.h"
 
 #include <algorithm>
 #include <limits>
 #include <set>
 
@@ skipping 68 matching lines @@
   }
   return false;
 }
 
 }  // namespace
 
 scoped_refptr<Picture> Picture::Create(gfx::Rect layer_rect) {
   return make_scoped_refptr(new Picture(layer_rect));
 }
 
-scoped_refptr<Picture> Picture::CreateFromValue(const base::Value* value) {
-  bool success;
-  scoped_refptr<Picture> picture =
-    make_scoped_refptr(new Picture(value, &success));
-  if (!success)
-    picture = NULL;
-  return picture;
-}
-
 Picture::Picture(gfx::Rect layer_rect)
     : layer_rect_(layer_rect) {
   // Instead of recording a trace event for object creation here, we wait for
   // the picture to be recorded in Picture::Record.
 }
 
-Picture::Picture(const base::Value* raw_value, bool* success) {
+scoped_refptr<Picture> Picture::CreateFromValue(const base::Value* raw_value) {
+  scoped_refptr<Picture> picture(NULL);
   const base::DictionaryValue* value = NULL;
   if (!raw_value->GetAsDictionary(&value)) {
-    *success = false;
-    return;
+    return picture;

[vmpstr, 2013/06/27 16:23:44]

Can you return NULL here explicitly and declare |picture| closer to where it's
being initialized?

[scroggo, 2013/06/27 16:45:45]

Done.

   }
 
   // Decode the picture from base64.
   std::string encoded;
   if (!value->GetString("skp64", &encoded)) {
-    *success = false;
-    return;
+    return picture;
   }
 
   std::string decoded;
   base::Base64Decode(encoded, &decoded);
   SkMemoryStream stream(decoded.data(), decoded.size());
 
-  const base::Value* layer_rect = NULL;
-  if (!value->Get("params.layer_rect", &layer_rect)) {
-    *success = false;
-    return;
-  }
-  if (!MathUtil::FromValue(layer_rect, &layer_rect_)) {
-    *success = false;
-    return;
+  const base::Value* layer_rect_value = NULL;
+  if (!value->Get("params.layer_rect", &layer_rect_value)) {
+    return picture;
   }
 
-  const base::Value* opaque_rect = NULL;
-  if (!value->Get("params.opaque_rect", &opaque_rect)) {
-    *success = false;
-    return;
+  gfx::Rect layer_rect;
+  if (!MathUtil::FromValue(layer_rect_value, &layer_rect)) {
+    return picture;
   }
-  if (!MathUtil::FromValue(opaque_rect, &opaque_rect_)) {
-    *success = false;
-    return;
+
+  const base::Value* opaque_rect_value = NULL;
+  if (!value->Get("params.opaque_rect", &opaque_rect_value)) {
+    return picture;
+  }
+
+  gfx::Rect opaque_rect;
+  if (!MathUtil::FromValue(opaque_rect_value, &opaque_rect)) {
+    return picture;
   }
 
   // Read the picture. This creates an empty picture on failure.
-  picture_ = skia::AdoptRef(new SkPicture(&stream, success, &DecodeBitmap));
+  picture = new Picture(SkPicture::CreateFromStream(&stream, &DecodeBitmap),

[vmpstr, 2013/06/27 16:23:44]

What does CreateFromStream return in case of failure? I'm a bit weary that if we
can't create the picture, then we still return a valid object (because of new
Picture). If CreateFromStream returns NULL, then we should return NULL.

[scroggo, 2013/06/27 16:45:45]

NULL.
Done.

+                        layer_rect, opaque_rect);
+  return picture;
+}
+
+Picture::Picture(SkPicture* picture,
+                 gfx::Rect layer_rect,
+                 gfx::Rect opaque_rect) :
+    layer_rect_(layer_rect),
+    opaque_rect_(opaque_rect),
+    picture_(skia::AdoptRef(picture)) {
 }
 
 Picture::Picture(const skia::RefPtr<SkPicture>& picture,
                  gfx::Rect layer_rect,
                  gfx::Rect opaque_rect,
                  const PixelRefMap& pixel_refs) :
     layer_rect_(layer_rect),
     opaque_rect_(opaque_rect),
     picture_(picture),
     pixel_refs_(pixel_refs) {
@@ skipping 294 matching lines @@
   raster_data->Set("picture_id", TracedValue::CreateIDRef(this).release());
   raster_data->SetDouble("scale", scale);
   raster_data->SetDouble("rect_x", rect.x());
   raster_data->SetDouble("rect_y", rect.y());
   raster_data->SetDouble("rect_width", rect.width());
   raster_data->SetDouble("rect_height", rect.height());
   return TracedValue::FromValue(raster_data.release());
 }
 
 }  // namespace cc