Allow filters to specify their initial visual rect.

cc/playback/display_item_list.h

 // 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.
 
 #ifndef CC_PLAYBACK_DISPLAY_ITEM_LIST_H_
 #define CC_PLAYBACK_DISPLAY_ITEM_LIST_H_
 
 #include <stddef.h>
 
 #include <memory>
@@ skipping 55 matching lines @@
   void Raster(SkCanvas* canvas, SkPicture::AbortCallback* callback) const;
 
   // This is a fast path for use only if canvas_ is set and
   // retain_individual_display_items_ is false. This method also updates
   // approximate_op_count_.
   void RasterIntoCanvas(const DisplayItem& display_item);
 
   // Because processing happens in these CreateAndAppend functions, all the set
   // up for the item should be done via the args, which is why the return type
   // needs to be const, to prevent set-after-processing mistakes.
+
+  // Most paired begin item types default to an empty visual rect, which will
+  // subsequently be grown as needed to encompass any contained items that draw
+  // content, such as drawing or filter items.
   template <typename DisplayItemType, typename... Args>
   const DisplayItemType& CreateAndAppendPairedBeginItem(Args&&... args) {
+    return CreateAndAppendPairedBeginItemWithVisualRect<DisplayItemType>(
+        gfx::Rect(), std::forward<Args>(args)...);
+  }
+
+  // This method variant is exposed to allow filters to specify their visual
+  // rect since they may draw content despite containing no drawing items.
+  template <typename DisplayItemType, typename... Args>
+  const DisplayItemType& CreateAndAppendPairedBeginItemWithVisualRect(

[vmpstr, 2016/08/11 23:39:37]

It seems like we could just have CreateAndAppendPairedBeginItem always take a
visual rect and require clients to specify it even if it's empty. Why do you
prefer this approach?

[wkorman, 2016/08/12 00:17:08]

Yeah, I thought about that. There are ~41 callsites and in all but one case
they'd be passing an empty rect. And I also liked the idea of making it more
explicit via a separate method name.

If we could use a default arg I would have done that, but I think we can't due
to the following varargs.

In any case I'm fine with switching to one method and passing empty rect in the
41 callsites (only 14 are outside of tests) if you'd prefer.

+      const gfx::Rect& visual_rect,
+      Args&&... args) {
     size_t item_index = inputs_.visual_rects.size();
-    inputs_.visual_rects.push_back(gfx::Rect());
+    inputs_.visual_rects.push_back(visual_rect);
     inputs_.begin_item_indices.push_back(item_index);
 
     return AllocateAndConstruct<DisplayItemType>(std::forward<Args>(args)...);
   }
 
   template <typename DisplayItemType, typename... Args>
   const DisplayItemType& CreateAndAppendPairedEndItem(Args&&... args) {
     DCHECK(!inputs_.begin_item_indices.empty());
     size_t last_begin_index = inputs_.begin_item_indices.back();
     inputs_.begin_item_indices.pop_back();
@@ skipping 129 matching lines @@
   Inputs inputs_;
 
   friend class base::RefCountedThreadSafe<DisplayItemList>;
   FRIEND_TEST_ALL_PREFIXES(DisplayItemListTest, ApproximateMemoryUsage);
   DISALLOW_COPY_AND_ASSIGN(DisplayItemList);
 };
 
 }  // namespace cc
 
 #endif  // CC_PLAYBACK_DISPLAY_ITEM_LIST_H_