Decouple Canvas from DisplayList and map Picture and PictureRecorder more directly to their Skia co…

sky/engine/core/painting/PictureRecorder.cpp

 // 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 "sky/engine/config.h"
+#include "sky/engine/core/painting/Canvas.h"
 #include "sky/engine/core/painting/PictureRecorder.h"
 
 #include "sky/engine/core/painting/Picture.h"
 
 namespace blink {
 
-PassRefPtr<PictureRecorder> PictureRecorder::create(double width, double height)
-{
-    return adoptRef(new PictureRecorder(FloatSize(width, height)));
-}
-
-PictureRecorder::PictureRecorder(const FloatSize& size)
-    : Canvas(size)
+PictureRecorder::PictureRecorder()
+    : m_pictureRecorder(adoptPtr(new SkPictureRecorder()))
 {
 }
 
 PictureRecorder::~PictureRecorder()
 {
 }
 
+bool PictureRecorder::isRecording() {
+    return m_canvas != nullptr && m_canvas->isRecording();

[eseidel, 2015/06/23 00:41:32]

Hmmm. you might be right, it's possible chromium style requires comparison to
nullptr/0.  Back in WebKit/Blink we would ahve just written return m_canvas &&
m_canvas->isRecording(), but maybe this is the correct chromium (and now sky)
style?

[abarth-chromium, 2015/06/23 02:28:44]

Chromium style doesn't have that rule.  I suspect Ian might just be noticing
that we use "!= null" a lot in Dart code because the Dart language requires it.

[iansf, 2015/06/23 22:46:09]

Oh -- originally this was just a check of m_canvas, and I felt like the code was
made more clear with the explicit nullptr comparison after the return, but if
that's not the style, I'm happy to remove the explicit check here and in the
other spot.

+}
+
+SkCanvas* PictureRecorder::beginRecording(double width, double height)
+{
+    return m_pictureRecorder->beginRecording(width, height);
+}
+
 PassRefPtr<Picture> PictureRecorder::endRecording()
 {
     if (!isRecording())
         return nullptr;
-    return Picture::create(finishRecording());
+    auto picture = Picture::create(

[eseidel, 2015/06/23 00:41:32]

As much as I like Auto, I don't think this is what you want.  PassRefPtrs
generally are bad news on the stack.  You want to get out of a PassRefPtr into a
RefPtr as soon as possible to avoid the auto-zeroing behavior.

RefPtr<Picture> picture = Picture::create();

return picture.release();

Would be better and would have fewer accidental clear gotchas.

[iansf, 2015/06/23 22:46:09]

Thanks for the explanation!  Done.

+        adoptRef(m_pictureRecorder->endRecording()));
+    m_canvas->clearSkCanvas();
+    m_canvas = nullptr;
+    return picture;
 }
 
+void PictureRecorder::set_canvas(PassRefPtr<Canvas> canvas) { m_canvas = canvas; }
+
 } // namespace blink