Reland "Add metrics for thumbnail memory usage"

This is a reland of f5d2f7eb

ThumbnailImage's data can be null. This adds a null check to
GetCompressedDataSizeInBytes() in which case it returns 0. This reland
also adds a unit test for this case.

Original change's description:
> Add metrics for thumbnail memory usage
>
> Adds two metrics logged every 5 minutes:
> * Per-thumbnail compressed data memory usage
> * Total compressed data memory usage
>
> The former has a sample for each thumbnail every 5 minutes, while the
> latter has exactly one every 5 minutes.
>
> Bug: 11261624
> Change-Id: I1595c6a6b771f73e3be9a5497d1cf6f6ec3d1af7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462685
> Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
> Reviewed-by: Dana Fried <dfried@chromium.org>
> Commit-Queue: Collin Baker <collinbaker@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#815714}

Bug: 11261624
Change-Id: I330f6a74041faf7ee1758e8b5f7a146456161f8c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2463510
Commit-Queue: Collin Baker <collinbaker@chromium.org>
Auto-Submit: Collin Baker <collinbaker@chromium.org>
Reviewed-by: Dana Fried <dfried@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816820}

chrome/browser/ui/BUILD.gn

@@ -1256,6 +1256,8 @@ static_library("ui") {
       "thumbnails/thumbnail_scheduler.h",
       "thumbnails/thumbnail_scheduler_impl.cc",
       "thumbnails/thumbnail_scheduler_impl.h",
+      "thumbnails/thumbnail_stats_tracker.cc",
+      "thumbnails/thumbnail_stats_tracker.h",
       "thumbnails/thumbnail_tab_helper.cc",
       "thumbnails/thumbnail_tab_helper.h",
       "toolbar/app_menu_icon_controller.cc",

chrome/browser/ui/thumbnails/thumbnail_image.cc

@@ -11,6 +11,7 @@
 #include "base/task/task_traits.h"
 #include "base/task/thread_pool.h"
 #include "base/time/time.h"
+#include "chrome/browser/ui/thumbnails/thumbnail_stats_tracker.h"
 #include "ui/gfx/codec/jpeg_codec.h"
 #include "ui/gfx/skia_util.h"
 
@@ -35,10 +36,12 @@ ThumbnailImage::ThumbnailImage(Delegate* delegate) : delegate_(delegate) {
   DCHECK(delegate_);
   DCHECK(!delegate_->thumbnail_);
   delegate_->thumbnail_ = this;
+  ThumbnailStatsTracker::GetInstance().AddThumbnail(this);
 }
 
 ThumbnailImage::~ThumbnailImage() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  ThumbnailStatsTracker::GetInstance().RemoveThumbnail(this);
   if (delegate_)
     delegate_->thumbnail_ = nullptr;
 }
@@ -88,6 +91,12 @@ void ThumbnailImage::RequestCompressedThumbnailData() {
     NotifyCompressedDataObservers(data_);
 }
 
+size_t ThumbnailImage::GetCompressedDataSizeInBytes() const {
+  if (!data_)
+    return 0;
+  return data_->data.size();
+}
+
 void ThumbnailImage::AssignJPEGData(base::TimeTicks assign_sk_bitmap_time,
                                     std::vector<uint8_t> data) {
   data_ = base::MakeRefCounted<base::RefCountedData<std::vector<uint8_t>>>(

chrome/browser/ui/thumbnails/thumbnail_image.h

@@ -94,6 +94,12 @@ class ThumbnailImage : public base::RefCounted<ThumbnailImage> {
   // Observer::OnCompressedThumbnailDataAvailable().
   void RequestCompressedThumbnailData();
 
+  // Returns the size of the compressed data backing this thumbnail.
+  // This size can be 0. Additionally, since this data is refcounted,
+  // it's possible this returns 0 even if the data is still allocated. A
+  // client can hold a reference to it after |this| drops its reference.
+  size_t GetCompressedDataSizeInBytes() const;
+
   void set_async_operation_finished_callback_for_testing(
       base::RepeatingClosure callback) {
     async_operation_finished_callback_ = std::move(callback);

chrome/browser/ui/thumbnails/thumbnail_stats_tracker.cc

+// Copyright 2020 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 "chrome/browser/ui/thumbnails/thumbnail_stats_tracker.h"
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/no_destructor.h"
+#include "base/time/time.h"
+#include "chrome/browser/ui/thumbnails/thumbnail_image.h"
+
+namespace {
+
+// Global instance. Only set once, except in tests which can reset the
+// instance and create a new one.
+ThumbnailStatsTracker* g_instance = nullptr;
+
+}  // namespace
+
+// static
+constexpr base::TimeDelta ThumbnailStatsTracker::kReportingInterval;
+
+// static
+ThumbnailStatsTracker& ThumbnailStatsTracker::GetInstance() {
+  if (!g_instance)
+    g_instance = new ThumbnailStatsTracker;
+  return *g_instance;
+}
+
+// static
+void ThumbnailStatsTracker::ResetInstanceForTesting() {
+  delete g_instance;
+  g_instance = nullptr;
+}
+
+ThumbnailStatsTracker::ThumbnailStatsTracker() {
+  heartbeat_timer_.Start(
+      FROM_HERE, kReportingInterval,
+      base::BindRepeating(&ThumbnailStatsTracker::RecordMetrics,
+                          base::Unretained(this)));
+}
+
+ThumbnailStatsTracker::~ThumbnailStatsTracker() {
+  // This is only called from tests. Make sure there are no thumbnails left.
+  DCHECK_EQ(thumbnails_.size(), 0u);
+}
+
+void ThumbnailStatsTracker::AddThumbnail(ThumbnailImage* thumbnail) {
+  auto result = thumbnails_.insert(thumbnail);
+  DCHECK(result.second) << "Thumbnail already added";
+}
+
+void ThumbnailStatsTracker::RemoveThumbnail(ThumbnailImage* thumbnail) {
+  int removed = thumbnails_.erase(thumbnail);
+  DCHECK_EQ(removed, 1) << "Thumbnail not added";
+}
+
+void ThumbnailStatsTracker::RecordMetrics() {
+  size_t total_size_bytes = 0;
+
+  for (ThumbnailImage* thumbnail : thumbnails_) {
+    size_t thumbnail_size_bytes = thumbnail->GetCompressedDataSizeInBytes();
+    total_size_bytes += thumbnail_size_bytes;
+
+    size_t thumbnail_size_kb = thumbnail_size_bytes / 1024;
+    UMA_HISTOGRAM_COUNTS_100(
+        "Tab.Preview.MemoryUsage.CompressedData.PerThumbnailKiB",
+        thumbnail_size_kb);
+  }
+
+  size_t total_size_kb = total_size_bytes / 1024;
+  UMA_HISTOGRAM_CUSTOM_COUNTS("Tab.Preview.MemoryUsage.CompressedData.TotalKiB",
+                              total_size_kb, 32, 8192, 50);
+}

chrome/browser/ui/thumbnails/thumbnail_stats_tracker.h

+// Copyright 2020 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 CHROME_BROWSER_UI_THUMBNAILS_THUMBNAIL_STATS_TRACKER_H_
+#define CHROME_BROWSER_UI_THUMBNAILS_THUMBNAIL_STATS_TRACKER_H_
+
+#include <set>
+
+#include "base/time/time.h"
+#include "base/timer/timer.h"
+
+namespace base {
+template <typename T>
+class NoDestructor;
+}  // namespace base
+
+class ThumbnailImage;
+
+// Records memory metrics across all thumbnails in a browser process.
+class ThumbnailStatsTracker {
+ private:
+  friend class ThumbnailImage;
+  friend class base::NoDestructor<ThumbnailStatsTracker>;
+
+  friend class ThumbnailStatsTrackerTest;
+
+  static constexpr base::TimeDelta kReportingInterval =
+      base::TimeDelta::FromMinutes(5);
+
+  // Gets the global instance for this process.
+  static ThumbnailStatsTracker& GetInstance();
+
+  // This must only be called if all registered thumbnails have been
+  // removed.
+  static void ResetInstanceForTesting();
+
+  ThumbnailStatsTracker();
+
+  // Exists only for ResetInstanceForTesting().
+  ~ThumbnailStatsTracker();
+
+  // Called from our friend, ThumbnailImage.
+  void AddThumbnail(ThumbnailImage* thumbnail);
+  void RemoveThumbnail(ThumbnailImage* thumbnail);
+
+  // Called by |heartbeat_timer_| to record metrics at a regular
+  // interval.
+  void RecordMetrics();
+
+  base::RepeatingTimer heartbeat_timer_;
+
+  std::set<ThumbnailImage*> thumbnails_;
+};
+
+#endif  // CHROME_BROWSER_UI_THUMBNAILS_THUMBNAIL_STATS_TRACKER_H_

chrome/browser/ui/thumbnails/thumbnail_stats_tracker_unittest.cc

+// Copyright 2020 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 "chrome/browser/ui/thumbnails/thumbnail_stats_tracker.h"
+
+#include "base/logging.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/rand_util.h"
+#include "base/run_loop.h"
+#include "base/test/metrics/histogram_tester.h"
+#include "base/test/task_environment.h"
+#include "base/time/time.h"
+#include "chrome/browser/ui/thumbnails/thumbnail_image.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+
+namespace {
+
+constexpr char kPerThumbnailMemoryUsageHistogram[] =
+    "Tab.Preview.MemoryUsage.CompressedData.PerThumbnailKiB";
+constexpr char kTotalMemoryUsageHistogram[] =
+    "Tab.Preview.MemoryUsage.CompressedData.TotalKiB";
+
+class StubThumbnailImageDelegate : public ThumbnailImage::Delegate {
+ public:
+  StubThumbnailImageDelegate() = default;
+  ~StubThumbnailImageDelegate() override = default;
+
+  // ThumbnailImage::Delegate:
+  void ThumbnailImageBeingObservedChanged(bool is_being_observed) override {}
+};
+
+class ThumbnailOwner : public ThumbnailImage::Delegate {
+ public:
+  ThumbnailOwner() = default;
+  ~ThumbnailOwner() override = default;
+
+  ThumbnailImage* Get() { return thumbnail_.get(); }
+
+  // ThumbnailImage::Delegate:
+  void ThumbnailImageBeingObservedChanged(bool is_being_observed) override {}
+
+ private:
+  scoped_refptr<ThumbnailImage> thumbnail_{
+      base::MakeRefCounted<ThumbnailImage>(this)};
+};
+
+}  // namespace
+
+class ThumbnailStatsTrackerTest : public ::testing::Test {
+ protected:
+  void SetUp() override {
+    // Delete previous instance and create new one to start the timer.
+    ThumbnailStatsTracker::ResetInstanceForTesting();
+    ThumbnailStatsTracker::GetInstance();
+  }
+
+  // A random bitmap will have roughly the same size, or even greater
+  // size, after compression.
+  SkBitmap CreateRandomBitmapOfSize(unsigned int width, unsigned int height) {
+    SkBitmap bitmap;
+    bitmap.allocN32Pixels(width, height);
+    for (unsigned int x = 0; x < width; ++x) {
+      for (unsigned int y = 0; y < height; ++y) {
+        *bitmap.getAddr32(x, y) = static_cast<uint32_t>(base::RandUint64());
+      }
+    }
+    return bitmap;
+  }
+
+  void AssignThumbnailBitmapAndWait(ThumbnailImage* thumbnail,
+                                    SkBitmap bitmap) {
+    base::RunLoop run_loop;
+    thumbnail->set_async_operation_finished_callback_for_testing(
+        run_loop.QuitClosure());
+    thumbnail->AssignSkBitmap(bitmap);
+    run_loop.Run();
+
+    // Clear the callback since the old one will be invalid.
+    thumbnail->set_async_operation_finished_callback_for_testing(
+        base::RepeatingClosure());
+  }
+
+  void AdvanceToNextReport() {
+    task_environment_.FastForwardBy(ThumbnailStatsTracker::kReportingInterval);
+  }
+
+  base::test::TaskEnvironment task_environment_{
+      base::test::TaskEnvironment::TimeSource::MOCK_TIME};
+
+  base::HistogramTester histogram_tester_;
+
+ private:
+  StubThumbnailImageDelegate stub_thumbnail_image_delegate_;
+};
+
+TEST_F(ThumbnailStatsTrackerTest, LogsMemoryMetricsAtHeartbeat) {
+  ThumbnailOwner thumbnail_1;
+  ThumbnailOwner thumbnail_2;
+
+  AssignThumbnailBitmapAndWait(thumbnail_1.Get(),
+                               CreateRandomBitmapOfSize(2, 2));
+  AssignThumbnailBitmapAndWait(thumbnail_2.Get(),
+                               CreateRandomBitmapOfSize(2, 2));
+
+  histogram_tester_.ExpectTotalCount(kPerThumbnailMemoryUsageHistogram, 0);
+  histogram_tester_.ExpectTotalCount(kTotalMemoryUsageHistogram, 0);
+
+  AdvanceToNextReport();
+  histogram_tester_.ExpectTotalCount(kPerThumbnailMemoryUsageHistogram, 2);
+  histogram_tester_.ExpectTotalCount(kTotalMemoryUsageHistogram, 1);
+
+  AdvanceToNextReport();
+  histogram_tester_.ExpectTotalCount(kPerThumbnailMemoryUsageHistogram, 4);
+  histogram_tester_.ExpectTotalCount(kTotalMemoryUsageHistogram, 2);
+}
+
+TEST_F(ThumbnailStatsTrackerTest, AlwaysLogsTotal) {
+  histogram_tester_.ExpectTotalCount(kPerThumbnailMemoryUsageHistogram, 0);
+  histogram_tester_.ExpectUniqueSample(kTotalMemoryUsageHistogram, 0, 0);
+
+  AdvanceToNextReport();
+  histogram_tester_.ExpectTotalCount(kPerThumbnailMemoryUsageHistogram, 0);
+  histogram_tester_.ExpectUniqueSample(kTotalMemoryUsageHistogram, 0, 1);
+
+  AdvanceToNextReport();
+  histogram_tester_.ExpectTotalCount(kPerThumbnailMemoryUsageHistogram, 0);
+  histogram_tester_.ExpectUniqueSample(kTotalMemoryUsageHistogram, 0, 2);
+}
+
+TEST_F(ThumbnailStatsTrackerTest, RecordedMemoryUsageIsCorrect) {
+  ThumbnailOwner thumbnail_1;
+  AssignThumbnailBitmapAndWait(thumbnail_1.Get(),
+                               CreateRandomBitmapOfSize(200, 150));
+  size_t thumbnail_1_size_kb =
+      thumbnail_1.Get()->GetCompressedDataSizeInBytes() / 1024;
+
+  AdvanceToNextReport();
+  histogram_tester_.ExpectBucketCount(kPerThumbnailMemoryUsageHistogram,
+                                      thumbnail_1_size_kb, 1);
+  histogram_tester_.ExpectBucketCount(kTotalMemoryUsageHistogram,
+                                      thumbnail_1_size_kb, 1);
+
+  ThumbnailOwner thumbnail_2;
+  AssignThumbnailBitmapAndWait(thumbnail_2.Get(),
+                               CreateRandomBitmapOfSize(100, 100));
+  size_t thumbnail_2_size_kb =
+      thumbnail_2.Get()->GetCompressedDataSizeInBytes() / 1024;
+
+  // This test won't work if the sizes are the same. While it's possible
+  // that the two randomly generated bitmaps will compress to the same
+  // size, the odds are astronomically low.
+  ASSERT_NE(thumbnail_1_size_kb, thumbnail_2_size_kb);
+
+  AdvanceToNextReport();
+  histogram_tester_.ExpectBucketCount(kPerThumbnailMemoryUsageHistogram,
+                                      thumbnail_1_size_kb, 2);
+  histogram_tester_.ExpectBucketCount(kPerThumbnailMemoryUsageHistogram,
+                                      thumbnail_2_size_kb, 1);
+  histogram_tester_.ExpectBucketCount(kTotalMemoryUsageHistogram,
+                                      thumbnail_1_size_kb, 1);
+  histogram_tester_.ExpectBucketCount(
+      kTotalMemoryUsageHistogram, thumbnail_1_size_kb + thumbnail_2_size_kb, 1);
+}
+
+TEST_F(ThumbnailStatsTrackerTest, EmptyThumbnailHasZeroSize) {
+  ThumbnailOwner thumbnail;
+  AdvanceToNextReport();
+  histogram_tester_.ExpectUniqueSample(kPerThumbnailMemoryUsageHistogram, 0, 1);
+  histogram_tester_.ExpectUniqueSample(kTotalMemoryUsageHistogram, 0, 1);
+}

chrome/test/BUILD.gn

@@ -4548,6 +4548,7 @@ test("unit_tests") {
       "../browser/ui/thumbnails/thumbnail_capture_driver_unittest.cc",
       "../browser/ui/thumbnails/thumbnail_image_unittest.cc",
       "../browser/ui/thumbnails/thumbnail_scheduler_impl_unittest.cc",
+      "../browser/ui/thumbnails/thumbnail_stats_tracker_unittest.cc",
       "../browser/ui/toolbar/app_menu_model_unittest.cc",
       "../browser/ui/toolbar/back_forward_menu_model_unittest.cc",
       "../browser/ui/toolbar/location_bar_model_unittest.cc",

tools/metrics/histograms/histograms_xml/tab/histograms.xml

@@ -159,6 +159,27 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   <summary>Records how a tab preview image was generated.</summary>
 </histogram>
 
+<histogram name="Tab.Preview.MemoryUsage.CompressedData.PerThumbnailKiB"
+    units="KB" expires_after="M92">
+  <owner>dfried@chromium.org</owner>
+  <owner>collinbaker@chromium.org</owner>
+  <summary>
+    Memory used by compressed thumbnails. Recorded once per thumbnail every 5
+    minutes.
+  </summary>
+</histogram>
+
+<histogram name="Tab.Preview.MemoryUsage.CompressedData.TotalKiB" units="KB"
+    expires_after="M92">
+  <owner>dfried@chromium.org</owner>
+  <owner>collinbaker@chromium.org</owner>
+  <summary>
+    Memory used by compressed thumbnails. Recorded once every 5 minutes at the
+    same time as Tab.Preview.MemoryUsage.CompressedData.PerThumbnailKiB. Each
+    sample is the sum of the corresponding samples in PerThumbnailKiB.
+  </summary>
+</histogram>
+
 <histogram name="Tab.Preview.TimeToFirstUsableFrameAfterStartCapture"
     units="ms" expires_after="2020-12-15">
   <owner>dfried@chromium.org</owner>