Removed instances of unnecessary values (eliminates superfluous

copies).  Adds linter check too.

Author: gaaclarke

.clang-tidy

@@ -10,7 +10,9 @@ readability-identifier-naming,\
 -clang-analyzer-nullability.NullPassedToNonnull,\
 -clang-analyzer-nullability.NullablePassedToNonnull,\
 -clang-analyzer-nullability.NullReturnedFromNonnull,\
--clang-analyzer-nullability.NullableReturnedFromNonnull"
+-clang-analyzer-nullability.NullableReturnedFromNonnull,\
+performance-move-const-arg,\
+performance-unnecessary-value-param"
 
 # Only warnings treated as errors are reported
 # in the "ci/lint.sh" script and pre-push git hook.
@@ -22,7 +24,9 @@ clang-analyzer-*,\
 readability-identifier-naming,\
 clang-diagnostic-*,\
 google-objc-*,\
-google-explicit-constructor"
+google-explicit-constructor,\
+performance-move-const-arg,\
+performance-unnecessary-value-param"
 
 CheckOptions:
   - key:    modernize-use-default-member-init.UseAssignment

common/graphics/persistent_cache.cc

@@ -8,6 +8,7 @@
 #include <memory>
 #include <string>
 #include <string_view>
+#include <utility>
 
 #include "flutter/fml/base32.h"
 #include "flutter/fml/file.h"
@@ -74,7 +75,7 @@ void PersistentCache::ResetCacheForProcess() {
 }
 
 void PersistentCache::SetCacheDirectoryPath(std::string path) {
-  cache_base_path_ = path;
+  cache_base_path_ = std::move(path);
 }
 
 bool PersistentCache::Purge() {
@@ -343,10 +344,11 @@ sk_sp<SkData> PersistentCache::load(const SkData& key) {
   return result;
 }
 
-static void PersistentCacheStore(fml::RefPtr<fml::TaskRunner> worker,
-                                 std::shared_ptr<fml::UniqueFD> cache_directory,
-                                 std::string key,
-                                 std::unique_ptr<fml::Mapping> value) {
+static void PersistentCacheStore(
+    const fml::RefPtr<fml::TaskRunner>& worker,
+    const std::shared_ptr<fml::UniqueFD>& cache_directory,
+    std::string key,
+    std::unique_ptr<fml::Mapping> value) {
   // The static leak checker gets confused by the use of fml::MakeCopyable.
   // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks)
   auto task = fml::MakeCopyable([cache_directory,             //
@@ -440,13 +442,13 @@ void PersistentCache::DumpSkp(const SkData& data) {
 }
 
 void PersistentCache::AddWorkerTaskRunner(
-    fml::RefPtr<fml::TaskRunner> task_runner) {
+    const fml::RefPtr<fml::TaskRunner>& task_runner) {
   std::scoped_lock lock(worker_task_runners_mutex_);
   worker_task_runners_.insert(task_runner);
 }
 
 void PersistentCache::RemoveWorkerTaskRunner(
-    fml::RefPtr<fml::TaskRunner> task_runner) {
+    const fml::RefPtr<fml::TaskRunner>& task_runner) {
   std::scoped_lock lock(worker_task_runners_mutex_);
   auto found = worker_task_runners_.find(task_runner);
   if (found != worker_task_runners_.end()) {
@@ -467,7 +469,7 @@ fml::RefPtr<fml::TaskRunner> PersistentCache::GetWorkerTaskRunner() const {
 
 void PersistentCache::SetAssetManager(std::shared_ptr<AssetManager> value) {
   TRACE_EVENT_INSTANT0("flutter", "PersistentCache::SetAssetManager");
-  asset_manager_ = value;
+  asset_manager_ = std::move(value);
 }
 
 std::vector<std::unique_ptr<fml::Mapping>>

common/graphics/persistent_cache.h

@@ -68,9 +68,9 @@ class PersistentCache : public GrContextOptions::PersistentCache {
 
   ~PersistentCache() override;
 
-  void AddWorkerTaskRunner(fml::RefPtr<fml::TaskRunner> task_runner);
+  void AddWorkerTaskRunner(const fml::RefPtr<fml::TaskRunner>& task_runner);
 
-  void RemoveWorkerTaskRunner(fml::RefPtr<fml::TaskRunner> task_runner);
+  void RemoveWorkerTaskRunner(const fml::RefPtr<fml::TaskRunner>& task_runner);
 
   // Whether Skia tries to store any shader into this persistent cache after
   // |ResetStoredNewShaders| is called. This flag is usually reset before each

common/graphics/texture.cc

@@ -16,7 +16,7 @@ Texture::~Texture() = default;
 
 TextureRegistry::TextureRegistry() = default;
 
-void TextureRegistry::RegisterTexture(std::shared_ptr<Texture> texture) {
+void TextureRegistry::RegisterTexture(const std::shared_ptr<Texture>& texture) {
   if (!texture) {
     return;
   }

common/graphics/texture.h

@@ -62,7 +62,7 @@ class TextureRegistry {
   TextureRegistry();
 
   // Called from raster thread.
-  void RegisterTexture(std::shared_ptr<Texture> texture);
+  void RegisterTexture(const std::shared_ptr<Texture>& texture);
 
   // Called from raster thread.
   void RegisterContextListener(uintptr_t id,

display_list/display_list_builder.cc

@@ -897,7 +897,7 @@ void DisplayListBuilder::drawImage(const sk_sp<DlImage> image,
       : Push<DrawImageOp>(0, 1, std::move(image), point, sampling);
   CheckLayerOpacityCompatibility(render_with_attributes);
 }
-void DisplayListBuilder::drawImage(const sk_sp<DlImage> image,
+void DisplayListBuilder::drawImage(const sk_sp<DlImage>& image,
                                    const SkPoint point,
                                    DlImageSampling sampling,
                                    const DlPaint* paint) {
@@ -919,7 +919,7 @@ void DisplayListBuilder::drawImageRect(const sk_sp<DlImage> image,
                         render_with_attributes, constraint);
   CheckLayerOpacityCompatibility(render_with_attributes);
 }
-void DisplayListBuilder::drawImageRect(const sk_sp<DlImage> image,
+void DisplayListBuilder::drawImageRect(const sk_sp<DlImage>& image,
                                        const SkRect& src,
                                        const SkRect& dst,
                                        DlImageSampling sampling,
@@ -944,7 +944,7 @@ void DisplayListBuilder::drawImageNine(const sk_sp<DlImage> image,
       : Push<DrawImageNineOp>(0, 1, std::move(image), center, dst, filter);
   CheckLayerOpacityCompatibility(render_with_attributes);
 }
-void DisplayListBuilder::drawImageNine(const sk_sp<DlImage> image,
+void DisplayListBuilder::drawImageNine(const sk_sp<DlImage>& image,
                                        const SkIRect& center,
                                        const SkRect& dst,
                                        DlFilterMode filter,
@@ -1017,7 +1017,7 @@ void DisplayListBuilder::drawAtlas(const sk_sp<DlImage> atlas,
   // of the transforms and texture rectangles.
   UpdateLayerOpacityCompatibility(false);
 }
-void DisplayListBuilder::drawAtlas(const sk_sp<DlImage> atlas,
+void DisplayListBuilder::drawAtlas(const sk_sp<DlImage>& atlas,
                                    const SkRSXform xform[],
                                    const SkRect tex[],
                                    const DlColor colors[],

display_list/display_list_builder.h

@@ -275,7 +275,7 @@ class DisplayListBuilder final : public virtual Dispatcher,
                  const SkPoint point,
                  DlImageSampling sampling,
                  bool render_with_attributes) override;
-  void drawImage(const sk_sp<DlImage> image,
+  void drawImage(const sk_sp<DlImage>& image,
                  const SkPoint point,
                  DlImageSampling sampling,
                  const DlPaint* paint = nullptr);
@@ -287,7 +287,7 @@ class DisplayListBuilder final : public virtual Dispatcher,
       bool render_with_attributes,
       SkCanvas::SrcRectConstraint constraint =
           SkCanvas::SrcRectConstraint::kFast_SrcRectConstraint) override;
-  void drawImageRect(const sk_sp<DlImage> image,
+  void drawImageRect(const sk_sp<DlImage>& image,
                      const SkRect& src,
                      const SkRect& dst,
                      DlImageSampling sampling,
@@ -299,7 +299,7 @@ class DisplayListBuilder final : public virtual Dispatcher,
                      const SkRect& dst,
                      DlFilterMode filter,
                      bool render_with_attributes) override;
-  void drawImageNine(const sk_sp<DlImage> image,
+  void drawImageNine(const sk_sp<DlImage>& image,
                      const SkIRect& center,
                      const SkRect& dst,
                      DlFilterMode filter,
@@ -318,7 +318,7 @@ class DisplayListBuilder final : public virtual Dispatcher,
                  DlImageSampling sampling,
                  const SkRect* cullRect,
                  bool render_with_attributes) override;
-  void drawAtlas(const sk_sp<DlImage> atlas,
+  void drawAtlas(const sk_sp<DlImage>& atlas,
                  const SkRSXform xform[],
                  const SkRect tex[],
                  const DlColor colors[],

display_list/display_list_canvas_unittests.cc

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <utility>
+
 #include "flutter/display_list/display_list.h"
 #include "flutter/display_list/display_list_canvas_dispatcher.h"
 #include "flutter/display_list/display_list_canvas_recorder.h"
@@ -239,7 +241,8 @@ static void EmptyDlRenderer(DisplayListBuilder&) {}
 
 class RenderSurface {
  public:
-  explicit RenderSurface(sk_sp<SkSurface> surface) : surface_(surface) {
+  explicit RenderSurface(sk_sp<SkSurface> surface)
+      : surface_(std::move(surface)) {
     EXPECT_EQ(canvas()->save(), 1);
   }
   ~RenderSurface() { sk_free(addr_); }
@@ -597,10 +600,10 @@ class TestParameters {
 class CaseParameters {
  public:
   explicit CaseParameters(std::string info)
-      : CaseParameters(info, EmptyCvRenderer, EmptyDlRenderer) {}
+      : CaseParameters(std::move(info), EmptyCvRenderer, EmptyDlRenderer) {}
 
-  CaseParameters(std::string info, CvSetup cv_setup, DlRenderer dl_setup)
-      : CaseParameters(info,
+  CaseParameters(std::string info, CvSetup& cv_setup, DlRenderer& dl_setup)
+      : CaseParameters(std::move(info),
                        cv_setup,
                        dl_setup,
                        EmptyCvRenderer,
@@ -611,15 +614,15 @@ class CaseParameters {
                        false) {}
 
   CaseParameters(std::string info,
-                 CvSetup cv_setup,
-                 DlRenderer dl_setup,
-                 CvRenderer cv_restore,
-                 DlRenderer dl_restore,
+                 CvSetup& cv_setup,
+                 DlRenderer& dl_setup,
+                 CvRenderer& cv_restore,
+                 DlRenderer& dl_restore,
                  DlColor bg,
                  bool has_diff_clip,
                  bool has_mutating_save_layer,
                  bool fuzzy_compare_components)
-      : info_(info),
+      : info_(std::move(info)),
         bg_(bg),
         cv_setup_(cv_setup),
         dl_setup_(dl_setup),
@@ -629,8 +632,8 @@ class CaseParameters {
         has_mutating_save_layer_(has_mutating_save_layer),
         fuzzy_compare_components_(fuzzy_compare_components) {}
 
-  CaseParameters with_restore(CvRenderer cv_restore,
-                              DlRenderer dl_restore,
+  CaseParameters with_restore(CvRenderer& cv_restore,
+                              DlRenderer& dl_restore,
                               bool mutating_layer,
                               bool fuzzy_compare_components = false) {
     return CaseParameters(info_, cv_setup_, dl_setup_, cv_restore, dl_restore,
@@ -1945,9 +1948,9 @@ class CanvasCompareTester {
   }
 
   static void checkGroupOpacity(const RenderEnvironment& env,
-                                sk_sp<DisplayList> display_list,
+                                const sk_sp<DisplayList>& display_list,
                                 const SkPixmap* ref_pixmap,
-                                const std::string info,
+                                const std::string& info,
                                 DlColor bg) {
     SkScalar opacity = 128.0 / 255.0;
 
@@ -2002,7 +2005,7 @@ class CanvasCompareTester {
 
   static void checkPixels(const SkPixmap* ref_pixels,
                           const SkRect ref_bounds,
-                          const std::string info,
+                          const std::string& info,
                           const DlColor bg) {
     uint32_t untouched = bg.premultipliedArgb();
     int pixels_touched = 0;
@@ -2026,7 +2029,7 @@ class CanvasCompareTester {
   static void quickCompareToReference(const SkPixmap* ref_pixels,
                                       const SkPixmap* test_pixels,
                                       bool should_match,
-                                      const std::string info) {
+                                      const std::string& info) {
     ASSERT_EQ(test_pixels->width(), ref_pixels->width()) << info;
     ASSERT_EQ(test_pixels->height(), ref_pixels->height()) << info;
     ASSERT_EQ(test_pixels->info().bytesPerPixel(), 4) << info;
@@ -2050,7 +2053,7 @@ class CanvasCompareTester {
 
   static void compareToReference(const SkPixmap* test_pixels,
                                  const SkPixmap* ref_pixels,
-                                 const std::string info,
+                                 const std::string& info,
                                  SkRect* bounds,
                                  const BoundsTolerance* tolerance,
                                  const DlColor bg,
@@ -2121,7 +2124,7 @@ class CanvasCompareTester {
     ASSERT_EQ(pixels_different, 0) << info;
   }
 
-  static void showBoundsOverflow(std::string info,
+  static void showBoundsOverflow(const std::string& info,
                                  SkIRect& bounds,
                                  const BoundsTolerance* tolerance,
                                  int pixLeft,
@@ -2186,7 +2189,7 @@ class CanvasCompareTester {
 
   static const DlImageColorSource kTestImageColorSource;
 
-  static sk_sp<SkTextBlob> MakeTextBlob(std::string string,
+  static sk_sp<SkTextBlob> MakeTextBlob(const std::string& string,
                                         SkScalar font_height) {
     SkFont font(SkTypeface::MakeFromName("ahem", SkFontStyle::Normal()),
                 font_height);

display_list/display_list_mask_filter_unittests.cc

@@ -131,33 +131,34 @@ void testNotEquals(DlMaskFilter* a, DlMaskFilter* b) {
   ASSERT_TRUE(NotEquals(b, a));
 }
 
-void testEquals(std::shared_ptr<const DlMaskFilter> a, DlMaskFilter* b) {
+void testEquals(const std::shared_ptr<const DlMaskFilter>& a, DlMaskFilter* b) {
   // a and b have the same nullness or values
   ASSERT_TRUE(Equals(a, b));
   ASSERT_FALSE(NotEquals(a, b));
   ASSERT_TRUE(Equals(b, a));
   ASSERT_FALSE(NotEquals(b, a));
 }
 
-void testNotEquals(std::shared_ptr<const DlMaskFilter> a, DlMaskFilter* b) {
+void testNotEquals(const std::shared_ptr<const DlMaskFilter>& a,
+                   DlMaskFilter* b) {
   // a and b do not have the same nullness or values
   ASSERT_FALSE(Equals(a, b));
   ASSERT_TRUE(NotEquals(a, b));
   ASSERT_FALSE(Equals(b, a));
   ASSERT_TRUE(NotEquals(b, a));
 }
 
-void testEquals(std::shared_ptr<const DlMaskFilter> a,
-                std::shared_ptr<const DlMaskFilter> b) {
+void testEquals(const std::shared_ptr<const DlMaskFilter>& a,
+                const std::shared_ptr<const DlMaskFilter>& b) {
   // a and b have the same nullness or values
   ASSERT_TRUE(Equals(a, b));
   ASSERT_FALSE(NotEquals(a, b));
   ASSERT_TRUE(Equals(b, a));
   ASSERT_FALSE(NotEquals(b, a));
 }
 
-void testNotEquals(std::shared_ptr<const DlMaskFilter> a,
-                   std::shared_ptr<const DlMaskFilter> b) {
+void testNotEquals(const std::shared_ptr<const DlMaskFilter>& a,
+                   const std::shared_ptr<const DlMaskFilter>& b) {
   // a and b do not have the same nullness or values
   ASSERT_FALSE(Equals(a, b));
   ASSERT_TRUE(NotEquals(a, b));

display_list/display_list_unittests.cc

@@ -5,6 +5,7 @@
 #include <memory>
 #include <string>
 #include <unordered_set>
+#include <utility>
 #include <vector>
 
 #include "flutter/display_list/display_list.h"
@@ -591,7 +592,7 @@ TEST(DisplayList, DisplayListTransformResetHandling) {
 }
 
 TEST(DisplayList, SingleOpsMightSupportGroupOpacityWithOrWithoutBlendMode) {
-  auto run_tests = [](std::string name,
+  auto run_tests = [](const std::string& name,
                       void build(DisplayListBuilder & builder),
                       bool expect_for_op, bool expect_with_kSrc) {
     {
@@ -783,12 +784,12 @@ class SaveLayerOptionsExpector : public virtual Dispatcher,
                                  public IgnoreTransformDispatchHelper,
                                  public IgnoreDrawDispatchHelper {
  public:
-  explicit SaveLayerOptionsExpector(SaveLayerOptions expected) {
+  explicit SaveLayerOptionsExpector(const SaveLayerOptions& expected) {
     expected_.push_back(expected);
   }
 
   explicit SaveLayerOptionsExpector(std::vector<SaveLayerOptions> expected)
-      : expected_(expected) {}
+      : expected_(std::move(expected)) {}
 
   void saveLayer(const SkRect* bounds,
                  const SaveLayerOptions options,
@@ -1541,10 +1542,10 @@ TEST(DisplayList, FlatDrawPointsProducesBounds) {
   }
 }
 
-static void test_rtree(sk_sp<const DlRTree> rtree,
+static void test_rtree(const sk_sp<const DlRTree>& rtree,
                        const SkRect& query,
                        std::vector<SkRect> expected_rects,
-                       std::vector<int> expected_indices) {
+                       const std::vector<int>& expected_indices) {
   std::vector<int> indices;
   rtree->search(query, &indices);
   EXPECT_EQ(indices, expected_indices);