Make View::Paint use ui::PaintRecorder to access PaintContext's canvas

ui/views/view.cc

Index: ui/views/view.cc
diff --git a/ui/views/view.cc b/ui/views/view.cc
index 9497dc5ed61b9b4ca24f15ee696b904a73595d16..387363a855348aa5dfc15801fc72b7a4d7b14a3f 100644
--- a/ui/views/view.cc
+++ b/ui/views/view.cc
@@ -23,6 +23,8 @@
 #include "ui/compositor/dip_util.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/layer_animator.h"
+#include "ui/compositor/paint_context.h"
+#include "ui/compositor/paint_recorder.h"
 #include "ui/events/event_target_iterator.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/geometry/point3_f.h"
@@ -731,7 +733,7 @@ void View::SchedulePaintInRect(const gfx::Rect& rect) {
   }
 }
 
-void View::Paint(const PaintContext& parent_context) {
+void View::Paint(const ui::PaintContext& parent_context) {
   if (!visible_)
     return;
 
@@ -742,7 +744,8 @@ void View::Paint(const PaintContext& parent_context) {
     DCHECK_IMPLIES(!parent(), bounds().origin() == gfx::Point());
     offset_to_parent = GetMirroredPosition().OffsetFromOrigin();
   }
-  PaintContext context = parent_context.CloneWithPaintOffset(offset_to_parent);
+  ui::PaintContext context =
+      parent_context.CloneWithPaintOffset(offset_to_parent);
 
   if (context.CanCheckInvalidated()) {
 #if DCHECK_IS_ON()
@@ -772,39 +775,40 @@ void View::Paint(const PaintContext& parent_context) {
 
   TRACE_EVENT1("views", "View::Paint", "class", GetClassName());
 
-  gfx::Canvas* canvas = context.canvas();
-  gfx::ScopedCanvas scoped_canvas(canvas);
-
   // If the view is backed by a layer, it should paint with itself as the origin
   // rather than relative to its parent.
-  if (!layer()) {
-    // Set the clip rect to the bounds of this View and translating the origin
-    // to the local bounds' top left point.
-    //
-    // Note that the X (or left) position we pass to ClipRectInt takes into
-    // consideration whether or not the view uses a right-to-left layout so that
-    // we paint our view in its mirrored position if need be.
+  bool paint_relative_to_parent = !layer();
+  if (paint_relative_to_parent) {
+    // Set the clip rect to the bounds of this View. Note that the X (or left)
+    // position we pass to ClipRect takes into consideration whether or not the
+    // View uses a right-to-left layout so that we paint the View in its
+    // mirrored position if need be.
+    ui::PaintRecorder recorder(context);
+    gfx::Canvas* canvas = recorder.canvas();
+    canvas->Save();
+
     gfx::Rect clip_rect = bounds();
     clip_rect.Inset(clip_insets_);
     if (parent_)
       clip_rect.set_x(parent_->GetMirroredXForRect(clip_rect));
     canvas->ClipRect(clip_rect);
-    if (canvas->IsClipEmpty())
-      return;
 
-    // Non-empty clip, translate the graphics such that 0,0 corresponds to where
-    // this view is located (related to its parent).
+    // Translate the graphics such that 0,0 corresponds to where
+    // this View is located relative to its parent.
     canvas->Translate(GetMirroredPosition().OffsetFromOrigin());
     canvas->Transform(GetTransform());
   }
 
   {
+    ui::PaintRecorder recorder(context);
+    gfx::Canvas* canvas = recorder.canvas();
+    gfx::ScopedCanvas scoped_canvas(canvas);
+
     // If the View we are about to paint requested the canvas to be flipped, we
     // should change the transform appropriately.
     // The canvas mirroring is undone once the View is done painting so that we
     // don't pass the canvas with the mirrored transform to Views that didn't
     // request the canvas to be flipped.
-    gfx::ScopedCanvas scoped(canvas);
     if (FlipCanvasOnPaintForRTLUI()) {
       canvas->Translate(gfx::Vector2d(width(), 0));
       canvas->Scale(-1, 1);
@@ -814,7 +818,13 @@ void View::Paint(const PaintContext& parent_context) {
     OnPaint(canvas);
   }
 
+  // View::Paint recursion over the subtree.
   PaintChildren(context);
+
+  if (paint_relative_to_parent) {
+    ui::PaintRecorder recorder(context);

[sky, 2015/04/02 23:43:09]

Why do you need this logic?

[danakj, 2015/04/03 00:27:36]

This matches the save on l.788. We can't use ScopedCanvas because recordings can
not be nested.

[sky, 2015/04/03 14:53:59]

Tricky, but error prone. How about a scoped_ptr<ScopedCanvas> defined on 780,
but set on 786 (where you have it now). I believe that would mean you can nuke
824-827.

[danakj, 2015/04/03 16:15:35]

The Canvas* here and at l786 are going to be different. Each scope of
PaintRecorder will have a different Canvas*.

One thing we could do is use a new RAII object that points at the paint context,
and makes a PaintRecorder and save/restores into it. But we need to cache the
display items in the correct order.. I think to do that, we need a longer lived
RAII object outside the scope of each PaintRecorder, and a ScopedSaveRestore
points at that. Nesting gets trickier than.

I agree it's more tricky with save/restore not on scope, but solutions around
this seem more complex than the problem. The issue is that the save and restore
have to be separate displayitems because there will be other displayitems in
between from children.

+    recorder.canvas()->Restore();
+  }
 }
 
 void View::set_background(Background* b) {
@@ -1339,7 +1349,7 @@ void View::NativeViewHierarchyChanged() {
 
 // Painting --------------------------------------------------------------------
 
-void View::PaintChildren(const PaintContext& context) {
+void View::PaintChildren(const ui::PaintContext& context) {
   TRACE_EVENT1("views", "View::PaintChildren", "class", GetClassName());
   for (int i = 0, count = child_count(); i < count; ++i)
     if (!child_at(i)->layer())
@@ -1449,12 +1459,12 @@ void View::UpdateChildLayerBounds(const gfx::Vector2d& offset) {
   }
 }
 
-void View::OnPaintLayer(gfx::Canvas* canvas) {
+void View::OnPaintLayer(const ui::PaintContext& context) {
   if (!layer()->fills_bounds_opaquely())
-    canvas->DrawColor(SK_ColorBLACK, SkXfermode::kClear_Mode);
+    context.canvas()->DrawColor(SK_ColorBLACK, SkXfermode::kClear_Mode);
   if (!visible_)
     return;
-  Paint(PaintContext(canvas, layer()->PaintRect()));
+  Paint(context);
 }
 
 void View::OnDelegatedFrameDamage(