Fix code smells reported by chrome's clang plugin

assets/asset_manager.h

@@ -19,7 +19,7 @@ class AssetManager final : public AssetResolver {
  public:
   AssetManager();
 
-  ~AssetManager();
+  ~AssetManager() override;
 
   void PushFront(std::unique_ptr<AssetResolver> resolver);
 

common/settings.cc

@@ -8,6 +8,10 @@
 
 namespace blink {
 
+Settings::Settings() = default;
+Settings::Settings(const Settings& other) = default;
+Settings::~Settings() = default;
+
 std::string Settings::ToString() const {
   std::stringstream stream;
   stream << "Settings: " << std::endl;

common/settings.h

@@ -22,6 +22,10 @@ using TaskObserverAdd =
 using TaskObserverRemove = std::function<void(intptr_t /* key */)>;
 
 struct Settings {
+  Settings();
+  Settings(const Settings& other);
+  ~Settings();
+
   // VM settings
   std::string vm_snapshot_data_path;
   std::string vm_snapshot_instr_path;

common/task_runners.cc

@@ -19,6 +19,8 @@ TaskRunners::TaskRunners(std::string label,
       ui_(std::move(ui)),
       io_(std::move(io)) {}
 
+TaskRunners::TaskRunners(const TaskRunners& other) = default;
+
 TaskRunners::~TaskRunners() = default;
 
 const std::string& TaskRunners::GetLabel() const {

common/task_runners.h

@@ -20,6 +20,8 @@ class TaskRunners {
               fml::RefPtr<fml::TaskRunner> ui,
               fml::RefPtr<fml::TaskRunner> io);
 
+  TaskRunners(const TaskRunners& other);
+
   ~TaskRunners();
 
   const std::string& GetLabel() const;

flow/BUILD.gn

@@ -8,6 +8,7 @@ source_set("flow") {
     "compositor_context.h",
     "debug_print.cc",
     "debug_print.h",
+    "embedded_views.cc",
     "embedded_views.h",
     "instrumentation.cc",
     "instrumentation.h",

flow/embedded_views.cc

@@ -0,0 +1,12 @@
+// Copyright 2013 The Flutter 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 "flutter/flow/embedded_views.h"
+
+namespace flow {
+
+bool ExternalViewEmbedder::SubmitFrame(GrContext* context) {
+  return false;
+};
+}  // namespace flow

flow/embedded_views.h

@@ -37,7 +37,7 @@ class ExternalViewEmbedder {
   virtual SkCanvas* CompositeEmbeddedView(int view_id,
                                           const EmbeddedViewParams& params) = 0;
 
-  virtual bool SubmitFrame(GrContext* context) { return false; };
+  virtual bool SubmitFrame(GrContext* context);
 
   virtual ~ExternalViewEmbedder() = default;
 

flow/layers/layer_tree.cc

@@ -96,7 +96,7 @@ sk_sp<SkPicture> LayerTree::Flatten(const SkRect& bounds) {
   TRACE_EVENT0("flutter", "LayerTree::Flatten");
 
   SkPictureRecorder recorder;
-  auto canvas = recorder.beginRecording(bounds);
+  auto* canvas = recorder.beginRecording(bounds);
 
   if (!canvas) {
     return nullptr;

flow/layers/picture_layer.cc

@@ -15,7 +15,7 @@ PictureLayer::~PictureLayer() = default;
 void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
   SkPicture* sk_picture = picture();
 
-  if (auto cache = context->raster_cache) {
+  if (auto* cache = context->raster_cache) {
     SkMatrix ctm = matrix;
     ctm.postTranslate(offset_.x(), offset_.y());
 #ifndef SUPPORT_FRACTIONAL_TRANSLATION

flow/raster_cache.cc

@@ -18,6 +18,16 @@
 
 namespace flow {
 
+RasterCacheResult::RasterCacheResult() {}
+
+RasterCacheResult::RasterCacheResult(const RasterCacheResult& other) = default;
+
+RasterCacheResult::~RasterCacheResult() = default;
+
+RasterCacheResult::RasterCacheResult(sk_sp<SkImage> image,
+                                     const SkRect& logical_rect)
+    : image_(std::move(image)), logical_rect_(logical_rect) {}
+
 void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const {
   SkAutoCanvasRestore auto_restore(&canvas, true);
   SkIRect bounds =

flow/raster_cache.h

@@ -19,10 +19,11 @@ namespace flow {
 
 class RasterCacheResult {
  public:
-  RasterCacheResult() {}
+  RasterCacheResult();
+  RasterCacheResult(const RasterCacheResult& other);
+  ~RasterCacheResult();
 
-  RasterCacheResult(sk_sp<SkImage> image, const SkRect& logical_rect)
-      : image_(std::move(image)), logical_rect_(logical_rect) {}
+  RasterCacheResult(sk_sp<SkImage> image, const SkRect& logical_rect);
 
   operator bool() const { return static_cast<bool>(image_); }
 

fml/BUILD.gn

@@ -56,6 +56,7 @@ source_set("fml") {
     "task_runner.h",
     "thread.cc",
     "thread.h",
+    "thread_local.cc",
     "thread_local.h",
     "time/time_delta.h",
     "time/time_point.cc",

fml/file.cc

@@ -43,4 +43,19 @@ fml::UniqueFD CreateDirectory(const fml::UniqueFD& base_directory,
   return CreateDirectory(base_directory, components, permission, 0);
 }
 
+ScopedTemporaryDirectory::ScopedTemporaryDirectory() {
+  path_ = CreateTemporaryDirectory();
+  if (path_ != "") {
+    dir_fd_ = OpenDirectory(path_.c_str(), false, FilePermission::kRead);
+  }
+}
+
+ScopedTemporaryDirectory::~ScopedTemporaryDirectory() {
+  if (path_ != "") {
+    if (!UnlinkDirectory(path_.c_str())) {
+      FML_LOG(ERROR) << "Could not remove directory: " << path_;
+    }
+  }
+}
+
 }  // namespace fml

fml/file.h

@@ -75,20 +75,9 @@ bool WriteAtomically(const fml::UniqueFD& base_directory,
 
 class ScopedTemporaryDirectory {
  public:
-  ScopedTemporaryDirectory() {
-    path_ = CreateTemporaryDirectory();
-    if (path_ != "") {
-      dir_fd_ = OpenDirectory(path_.c_str(), false, FilePermission::kRead);
-    }
-  }
-
-  ~ScopedTemporaryDirectory() {
-    if (path_ != "") {
-      if (!UnlinkDirectory(path_.c_str())) {
-        FML_LOG(ERROR) << "Could not remove directory: " << path_;
-      }
-    }
-  }
+  ScopedTemporaryDirectory();
+
+  ~ScopedTemporaryDirectory();
 
   const UniqueFD& fd() { return dir_fd_; }
 

fml/logging.cc

@@ -34,7 +34,7 @@ const char* StripDots(const char* path) {
 }
 
 const char* StripPath(const char* path) {
-  auto p = strrchr(path, '/');
+  auto* p = strrchr(path, '/');
   if (p)
     return p + 1;
   else

fml/mapping.cc

@@ -22,4 +22,12 @@ const uint8_t* DataMapping::GetMapping() const {
   return data_.data();
 }
 
+size_t NonOwnedMapping::GetSize() const {
+  return size_;
+}
+
+const uint8_t* NonOwnedMapping::GetMapping() const {
+  return data_;
+}
+
 }  // namespace fml

fml/mapping.h

@@ -84,9 +84,9 @@ class NonOwnedMapping : public Mapping {
   NonOwnedMapping(const uint8_t* data, size_t size)
       : data_(data), size_(size) {}
 
-  size_t GetSize() const override { return size_; }
+  size_t GetSize() const override;
 
-  const uint8_t* GetMapping() const override { return data_; }
+  const uint8_t* GetMapping() const override;
 
  private:
   const uint8_t* const data_;

fml/message.cc

@@ -8,6 +8,10 @@
 
 namespace fml {
 
+size_t MessageSerializable::GetSerializableTag() const {
+  return 0;
+};
+
 Message::Message() = default;
 
 Message::~Message() = default;
@@ -96,7 +100,7 @@ uint8_t* Message::PrepareDecode(size_t size) {
   if ((size + size_read_) > buffer_length_) {
     return nullptr;
   }
-  auto buffer = buffer_ + size_read_;
+  auto* buffer = buffer_ + size_read_;
   size_read_ += size;
   return buffer;
 }

fml/message.h

@@ -47,7 +47,7 @@ class MessageSerializable {
 
   virtual bool Deserialize(Message& message) = 0;
 
-  virtual size_t GetSerializableTag() const { return 0; };
+  virtual size_t GetSerializableTag() const;
 };
 
 // The traits passed to the encode/decode calls that accept traits should be
@@ -88,7 +88,7 @@ class Message {
   template <typename T,
             typename = std::enable_if_t<std::is_trivially_copyable<T>::value>>
   FML_WARN_UNUSED_RESULT bool Encode(const T& value) {
-    if (auto buffer = PrepareEncode(sizeof(T))) {
+    if (auto* buffer = PrepareEncode(sizeof(T))) {
       ::memcpy(buffer, &value, sizeof(T));
       return true;
     }
@@ -131,7 +131,7 @@ class Message {
   template <typename T,
             typename = std::enable_if_t<std::is_trivially_copyable<T>::value>>
   FML_WARN_UNUSED_RESULT bool Decode(T& value) {
-    if (auto buffer = PrepareDecode(sizeof(T))) {
+    if (auto* buffer = PrepareDecode(sizeof(T))) {
       ::memcpy(&value, buffer, sizeof(T));
       return true;
     }

fml/message_loop.cc

@@ -19,7 +19,7 @@ FML_THREAD_LOCAL ThreadLocal tls_message_loop([](intptr_t value) {
 });
 
 MessageLoop& MessageLoop::GetCurrent() {
-  auto loop = reinterpret_cast<MessageLoop*>(tls_message_loop.Get());
+  auto* loop = reinterpret_cast<MessageLoop*>(tls_message_loop.Get());
   FML_CHECK(loop != nullptr)
       << "MessageLoop::EnsureInitializedForCurrentThread was not called on "
          "this thread prior to message loop use.";

fml/message_loop_impl.cc

@@ -145,4 +145,13 @@ void MessageLoopImpl::RunExpiredTasks() {
   }
 }
 
+MessageLoopImpl::DelayedTask::DelayedTask(size_t p_order,
+                                          fml::closure p_task,
+                                          fml::TimePoint p_target_time)
+    : order(p_order), task(std::move(p_task)), target_time(p_target_time) {}
+
+MessageLoopImpl::DelayedTask::DelayedTask(const DelayedTask& other) = default;
+
+MessageLoopImpl::DelayedTask::~DelayedTask() = default;
+
 }  // namespace fml

fml/message_loop_impl.h

@@ -57,8 +57,9 @@ class MessageLoopImpl : public fml::RefCountedThreadSafe<MessageLoopImpl> {
 
     DelayedTask(size_t p_order,
                 fml::closure p_task,
-                fml::TimePoint p_target_time)
-        : order(p_order), task(std::move(p_task)), target_time(p_target_time) {}
+                fml::TimePoint p_target_time);
+    DelayedTask(const DelayedTask& other);
+    ~DelayedTask();
   };
 
   struct DelayedTaskCompare {

fml/platform/posix/file_posix.cc

@@ -19,7 +19,7 @@ namespace fml {
 
 std::string CreateTemporaryDirectory() {
   char directory_name[] = "/tmp/flutter_XXXXXXXX";
-  auto result = ::mkdtemp(directory_name);
+  auto* result = ::mkdtemp(directory_name);
   if (result == nullptr) {
     return "";
   }

fml/platform/posix/mapping_posix.cc

@@ -69,7 +69,7 @@ FileMapping::FileMapping(const fml::UniqueFD& handle,
 
   const auto is_writable = IsWritable(protection);
 
-  auto mapping =
+  auto* mapping =
       ::mmap(nullptr, stat_buffer.st_size, ToPosixProtectionFlags(protection),
              is_writable ? MAP_SHARED : MAP_PRIVATE, handle.get(), 0);
 

fml/platform/posix/native_library_posix.cc

@@ -58,7 +58,7 @@ fml::RefPtr<NativeLibrary> NativeLibrary::CreateForCurrentProcess() {
 }
 
 const uint8_t* NativeLibrary::ResolveSymbol(const char* symbol) {
-  auto resolved_symbol = static_cast<const uint8_t*>(::dlsym(handle_, symbol));
+  auto* resolved_symbol = static_cast<const uint8_t*>(::dlsym(handle_, symbol));
   if (resolved_symbol == nullptr) {
     FML_DLOG(INFO) << "Could not resolve symbol in library: " << symbol;
   }

fml/string_view.cc

@@ -84,7 +84,7 @@ size_t StringView::find(StringView s, size_t pos) const {
   if (s.empty())
     return pos;
 
-  auto result = std::search(begin() + pos, end(), s.begin(), s.end());
+  auto* result = std::search(begin() + pos, end(), s.begin(), s.end());
   if (result == end())
     return npos;
   return result - begin();
@@ -94,7 +94,7 @@ size_t StringView::find(char c, size_t pos) const {
   if (pos > size_)
     return npos;
 
-  auto result = std::find(begin() + pos, end(), c);
+  auto* result = std::find(begin() + pos, end(), c);
   if (result == end())
     return npos;
   return result - begin();
@@ -106,8 +106,8 @@ size_t StringView::rfind(StringView s, size_t pos) const {
   if (s.empty())
     return std::min(pos, size_);
 
-  auto last = begin() + std::min(size_ - s.size(), pos) + s.size();
-  auto result = std::find_end(begin(), last, s.begin(), s.end());
+  auto* last = begin() + std::min(size_ - s.size(), pos) + s.size();
+  auto* result = std::find_end(begin(), last, s.begin(), s.end());
   if (result == last)
     return npos;
   return result - begin();

fml/thread_local.cc

@@ -0,0 +1,36 @@
+// Copyright 2013 The Flutter 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 "flutter/fml/thread_local.h"
+
+namespace fml {
+
+ThreadLocal::ThreadLocal() : ThreadLocal(nullptr) {}
+
+ThreadLocal::ThreadLocal(ThreadLocalDestroyCallback destroy)
+    : destroy_(destroy) {
+  auto callback =
+      reinterpret_cast<void (*)(void*)>(&ThreadLocal::ThreadLocalDestroy);
+  FML_CHECK(pthread_key_create(&_key, callback) == 0);
+}
+
+ThreadLocal::~ThreadLocal() {
+  // This will NOT call the destroy callbacks on thread local values still
+  // active in other threads. Those must be cleared manually. The usage
+  // of this class should be similar to the thread_local keyword but with
+  // with a static storage specifier
+
+  // Collect the container
+  delete reinterpret_cast<Box*>(pthread_getspecific(_key));
+
+  // Finally, collect the key
+  FML_CHECK(pthread_key_delete(_key) == 0);
+}
+
+ThreadLocal::Box::Box(ThreadLocalDestroyCallback destroy, intptr_t value)
+    : destroy_(destroy), value_(value) {}
+
+ThreadLocal::Box::~Box() = default;
+
+}  // namespace fml