Make DarkModeImageClassifier apis to work with SkPixmap.

For RSDM (wip patch crrev.com/c/2295379), the ImageDecodeCache expects
DarkModeFilter work on SkPixmap. This patch makes
DarkModeImageClassifier use SkPixmap instead of PaintImage. Also as
RSDM requires the purging of the cache to be implemented in decoded
image cache, this patch moves dark mode image cache outside dark mode
module. Now blink implements caching in Image class and dark mode
filter helper is added to handle cache modifications.

To check the correctness of the dark mode classifier logic to work on
full image than portion of the image, few more tests related to image
sprite and block samples are added.

Bug: 1094005
Change-Id: I39dbd7638691da459b898045d4721069dc5dd21c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2382990Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Prashant Nevase <prashant.n@samsung.com>
Cr-Commit-Position: refs/heads/master@{#809969}

third_party/blink/renderer/platform/BUILD.gn

@@ -879,10 +879,14 @@ component("platform") {
     "graphics/dark_mode_color_filter.h",
     "graphics/dark_mode_filter.cc",
     "graphics/dark_mode_filter.h",
+    "graphics/dark_mode_filter_helper.cc",
+    "graphics/dark_mode_filter_helper.h",
+    "graphics/dark_mode_image_cache.h",
     "graphics/dark_mode_image_classifier.cc",
     "graphics/dark_mode_image_classifier.h",
     "graphics/dark_mode_lab_color_space.h",
     "graphics/dark_mode_settings.h",
+    "graphics/dark_mode_types.h",
     "graphics/darkmode/darkmode_classifier.cc",
     "graphics/darkmode/darkmode_classifier.h",
     "graphics/dash_array.h",
@@ -1949,6 +1953,7 @@ source_set("blink_platform_unittests_sources") {
     "graphics/contiguous_container_test.cc",
     "graphics/dark_mode_color_classifier_test.cc",
     "graphics/dark_mode_filter_test.cc",
+    "graphics/dark_mode_image_cache_test.cc",
     "graphics/dark_mode_image_classifier_test.cc",
     "graphics/dark_mode_lab_color_space_test.cc",
     "graphics/decoding_image_generator_test.cc",

third_party/blink/renderer/platform/graphics/dark_mode_color_classifier.cc

@@ -5,7 +5,6 @@
 #include "third_party/blink/renderer/platform/graphics/dark_mode_color_classifier.h"
 
 #include "base/check_op.h"
-#include "third_party/blink/renderer/platform/graphics/graphics_types.h"
 
 namespace blink {
 namespace {
@@ -14,36 +13,34 @@ class SimpleColorClassifier : public DarkModeColorClassifier {
  public:
   static std::unique_ptr<SimpleColorClassifier> NeverInvert() {
     return std::unique_ptr<SimpleColorClassifier>(
-        new SimpleColorClassifier(DarkModeClassification::kDoNotApplyFilter));
+        new SimpleColorClassifier(DarkModeResult::kDoNotApplyFilter));
   }
 
   static std::unique_ptr<SimpleColorClassifier> AlwaysInvert() {
     return std::unique_ptr<SimpleColorClassifier>(
-        new SimpleColorClassifier(DarkModeClassification::kApplyFilter));
+        new SimpleColorClassifier(DarkModeResult::kApplyFilter));
   }
 
-  DarkModeClassification ShouldInvertColor(SkColor color) override {
-    return value_;
-  }
+  DarkModeResult ShouldInvertColor(SkColor color) override { return value_; }
 
  private:
-  SimpleColorClassifier(DarkModeClassification value) : value_(value) {}
+  explicit SimpleColorClassifier(DarkModeResult value) : value_(value) {}
 
-  DarkModeClassification value_;
+  DarkModeResult value_;
 };
 
 class InvertLowBrightnessColorsClassifier : public DarkModeColorClassifier {
  public:
-  InvertLowBrightnessColorsClassifier(int brightness_threshold)
+  explicit InvertLowBrightnessColorsClassifier(int brightness_threshold)
       : brightness_threshold_(brightness_threshold) {
     DCHECK_GT(brightness_threshold_, 0);
     DCHECK_LT(brightness_threshold_, 256);
   }
 
-  DarkModeClassification ShouldInvertColor(SkColor color) override {
+  DarkModeResult ShouldInvertColor(SkColor color) override {
     if (CalculateColorBrightness(color) < brightness_threshold_)
-      return DarkModeClassification::kApplyFilter;
-    return DarkModeClassification::kDoNotApplyFilter;
+      return DarkModeResult::kApplyFilter;
+    return DarkModeResult::kDoNotApplyFilter;
   }
 
  private:
@@ -52,16 +49,16 @@ class InvertLowBrightnessColorsClassifier : public DarkModeColorClassifier {
 
 class InvertHighBrightnessColorsClassifier : public DarkModeColorClassifier {
  public:
-  InvertHighBrightnessColorsClassifier(int brightness_threshold)
+  explicit InvertHighBrightnessColorsClassifier(int brightness_threshold)
       : brightness_threshold_(brightness_threshold) {
     DCHECK_GT(brightness_threshold_, 0);
     DCHECK_LT(brightness_threshold_, 256);
   }
 
-  DarkModeClassification ShouldInvertColor(SkColor color) override {
+  DarkModeResult ShouldInvertColor(SkColor color) override {
     if (CalculateColorBrightness(color) > brightness_threshold_)
-      return DarkModeClassification::kApplyFilter;
-    return DarkModeClassification::kDoNotApplyFilter;
+      return DarkModeResult::kApplyFilter;
+    return DarkModeResult::kDoNotApplyFilter;
   }
 
  private:

third_party/blink/renderer/platform/graphics/dark_mode_color_classifier.h

@@ -8,8 +8,9 @@
 #include <memory>
 
 #include "third_party/blink/renderer/platform/graphics/dark_mode_settings.h"
-#include "third_party/blink/renderer/platform/graphics/graphics_types.h"
+#include "third_party/blink/renderer/platform/graphics/dark_mode_types.h"
 #include "third_party/blink/renderer/platform/platform_export.h"
+#include "third_party/skia/include/core/SkColor.h"
 
 namespace blink {
 
@@ -29,7 +30,7 @@ class PLATFORM_EXPORT DarkModeColorClassifier {
   // whether to invert a color. The background is likely to be dark, so a lower
   // opacity will usually decrease the effective brightness of both the original
   // and the inverted colors.
-  virtual DarkModeClassification ShouldInvertColor(SkColor color) = 0;
+  virtual DarkModeResult ShouldInvertColor(SkColor color) = 0;
 };
 
 }  // namespace blink

third_party/blink/renderer/platform/graphics/dark_mode_color_classifier_test.cc

@@ -7,7 +7,6 @@
 #include "base/check_op.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/renderer/platform/graphics/dark_mode_settings.h"
-#include "third_party/blink/renderer/platform/graphics/graphics_types.h"
 #include "third_party/skia/include/core/SkColor.h"
 
 namespace blink {
@@ -33,18 +32,18 @@ TEST(DarkModeColorClassifierTest, ApplyFilterToDarkTextOnly) {
   //   * white text
   //   * text brighter than the text brightness threshold
   //   * text at the brightness threshold
-  EXPECT_EQ(DarkModeClassification::kApplyFilter,
+  EXPECT_EQ(DarkModeResult::kApplyFilter,
             classifier->ShouldInvertColor(GetColorWithBrightness(
                 settings.text_brightness_threshold - 5)));
-  EXPECT_EQ(DarkModeClassification::kApplyFilter,
+  EXPECT_EQ(DarkModeResult::kApplyFilter,
             classifier->ShouldInvertColor(SK_ColorBLACK));
 
-  EXPECT_EQ(DarkModeClassification::kDoNotApplyFilter,
+  EXPECT_EQ(DarkModeResult::kDoNotApplyFilter,
             classifier->ShouldInvertColor(SK_ColorWHITE));
-  EXPECT_EQ(DarkModeClassification::kDoNotApplyFilter,
+  EXPECT_EQ(DarkModeResult::kDoNotApplyFilter,
             classifier->ShouldInvertColor(GetColorWithBrightness(
                 settings.text_brightness_threshold + 5)));
-  EXPECT_EQ(DarkModeClassification::kDoNotApplyFilter,
+  EXPECT_EQ(DarkModeResult::kDoNotApplyFilter,
             classifier->ShouldInvertColor(
                 GetColorWithBrightness(settings.text_brightness_threshold)));
 }
@@ -56,18 +55,18 @@ TEST(DarkModeColorClassifierTest, ApplyFilterToLightBackgroundElementsOnly) {
   auto classifier =
       DarkModeColorClassifier::MakeBackgroundColorClassifier(settings);
 
-  EXPECT_EQ(DarkModeClassification::kApplyFilter,
+  EXPECT_EQ(DarkModeResult::kApplyFilter,
             classifier->ShouldInvertColor(SK_ColorWHITE));
-  EXPECT_EQ(DarkModeClassification::kDoNotApplyFilter,
+  EXPECT_EQ(DarkModeResult::kDoNotApplyFilter,
             classifier->ShouldInvertColor(SK_ColorBLACK));
 
-  EXPECT_EQ(DarkModeClassification::kApplyFilter,
+  EXPECT_EQ(DarkModeResult::kApplyFilter,
             classifier->ShouldInvertColor(GetColorWithBrightness(
                 settings.background_brightness_threshold + 5)));
-  EXPECT_EQ(DarkModeClassification::kDoNotApplyFilter,
+  EXPECT_EQ(DarkModeResult::kDoNotApplyFilter,
             classifier->ShouldInvertColor(GetColorWithBrightness(
                 settings.background_brightness_threshold)));
-  EXPECT_EQ(DarkModeClassification::kDoNotApplyFilter,
+  EXPECT_EQ(DarkModeResult::kDoNotApplyFilter,
             classifier->ShouldInvertColor(GetColorWithBrightness(
                 settings.background_brightness_threshold - 5)));
 }

third_party/blink/renderer/platform/graphics/dark_mode_filter.cc

@@ -13,7 +13,6 @@
 #include "third_party/blink/renderer/platform/graphics/dark_mode_color_filter.h"
 #include "third_party/blink/renderer/platform/graphics/dark_mode_image_classifier.h"
 #include "third_party/blink/renderer/platform/graphics/graphics_context.h"
-#include "third_party/blink/renderer/platform/graphics/graphics_types.h"
 #include "third_party/blink/renderer/platform/wtf/hash_functions.h"
 #include "third_party/blink/renderer/platform/wtf/lru_cache.h"
 #include "third_party/skia/include/core/SkColorFilter.h"
@@ -148,42 +147,47 @@ SkColor DarkModeFilter::InvertColorIfNeeded(SkColor color, ElementRole role) {
   return color;
 }
 
-bool DarkModeFilter::AnalyzeShouldApplyToImage(const SkRect& src,
-                                               const SkRect& dst) {
+DarkModeResult DarkModeFilter::AnalyzeShouldApplyToImage(
+    const SkRect& src,
+    const SkRect& dst) const {
   if (settings().image_policy == DarkModeImagePolicy::kFilterNone)
-    return false;
+    return DarkModeResult::kDoNotApplyFilter;
 
   if (settings().image_policy == DarkModeImagePolicy::kFilterAll)
-    return true;
+    return DarkModeResult::kApplyFilter;
 
   // Images being drawn from very smaller |src| rect, i.e. one of the dimensions
   // is very small, can be used for the border around the content or showing
   // separator. Consider these images irrespective of size of the rect being
   // drawn to. Classifying them will not be too costly.
   if (src.width() <= kMinImageLength || src.height() <= kMinImageLength)
-    return true;
+    return DarkModeResult::kNotClassified;
 
   // Do not consider images being drawn into bigger rect as these images are not
   // meant for icons or representing smaller widgets. These images are
   // considered as photos which should be untouched.
-  return (dst.width() <= kMaxImageLength && dst.height() <= kMaxImageLength);
+  return (dst.width() <= kMaxImageLength && dst.height() <= kMaxImageLength)
+             ? DarkModeResult::kNotClassified
+             : DarkModeResult::kDoNotApplyFilter;
 }
 
-void DarkModeFilter::ApplyToImageFlagsIfNeeded(const SkRect& src,
-                                               const SkRect& dst,
-                                               const PaintImage& paint_image,
-                                               cc::PaintFlags* flags) {
-  // The construction of |paint_image| is expensive, so ensure
-  // IsDarkModeActive() is checked prior to calling this function.
-  // See: https://crbug.com/1094781.
-  DCHECK(IsDarkModeActive());
-
-  if (!image_filter_ || !AnalyzeShouldApplyToImage(src, dst))
-    return;
+sk_sp<SkColorFilter> DarkModeFilter::ApplyToImage(const SkPixmap& pixmap,
+                                                  const SkRect& src,
+                                                  const SkRect& dst) {
+  DCHECK(AnalyzeShouldApplyToImage(src, dst) == DarkModeResult::kNotClassified);
+  DCHECK(settings().image_policy == DarkModeImagePolicy::kFilterSmart);
+  DCHECK(image_filter_);
+
+  return (image_classifier_->Classify(pixmap, src) ==
+          DarkModeResult::kApplyFilter)
+             ? image_filter_
+             : nullptr;
+}
 
-  if (ClassifyImage(settings(), src, dst, paint_image) ==
-      DarkModeClassification::kApplyFilter)
-    flags->setColorFilter(image_filter_);
+sk_sp<SkColorFilter> DarkModeFilter::GetImageFilter() {
+  DCHECK(settings().image_policy == DarkModeImagePolicy::kFilterAll);
+  DCHECK(image_filter_);
+  return image_filter_;
 }
 
 base::Optional<cc::PaintFlags> DarkModeFilter::ApplyToFlagsIfNeeded(
@@ -219,18 +223,18 @@ bool DarkModeFilter::ShouldApplyToColor(SkColor color, ElementRole role) {
     case ElementRole::kText:
       DCHECK(text_classifier_);
       return text_classifier_->ShouldInvertColor(color) ==
-             DarkModeClassification::kApplyFilter;
+             DarkModeResult::kApplyFilter;
     case ElementRole::kListSymbol:
       // TODO(prashant.n): Rename text_classifier_ to foreground_classifier_,
       // so that same classifier can be used for all roles which are supposed
       // to be at foreground.
       DCHECK(text_classifier_);
       return text_classifier_->ShouldInvertColor(color) ==
-             DarkModeClassification::kApplyFilter;
+             DarkModeResult::kApplyFilter;
     case ElementRole::kBackground:
       DCHECK(background_classifier_);
       return background_classifier_->ShouldInvertColor(color) ==
-             DarkModeClassification::kApplyFilter;
+             DarkModeResult::kApplyFilter;
     case ElementRole::kSVG:
       // 1) Inline SVG images are considered as individual shapes and do not
       // have an Image object associated with them. So they do not go through
@@ -251,23 +255,6 @@ size_t DarkModeFilter::GetInvertedColorCacheSizeForTesting() {
   return inverted_color_cache_->size();
 }
 
-DarkModeClassification DarkModeFilter::ClassifyImage(
-    const DarkModeSettings& settings,
-    const SkRect& src,
-    const SkRect& dst,
-    const PaintImage& paint_image) {
-  switch (settings.image_policy) {
-    case DarkModeImagePolicy::kFilterSmart:
-      return image_classifier_->Classify(paint_image, src, dst);
-    case DarkModeImagePolicy::kFilterNone:
-      return DarkModeClassification::kDoNotApplyFilter;
-    case DarkModeImagePolicy::kFilterAll:
-      return DarkModeClassification::kApplyFilter;
-  }
-
-  NOTREACHED();
-}
-
 ScopedDarkModeElementRoleOverride::ScopedDarkModeElementRoleOverride(
     GraphicsContext* graphics_context,
     DarkModeFilter::ElementRole role)

third_party/blink/renderer/platform/graphics/dark_mode_filter.h

@@ -8,12 +8,9 @@
 #include <memory>
 
 #include "cc/paint/paint_flags.h"
-#include "third_party/blink/renderer/platform/graphics/color.h"
 #include "third_party/blink/renderer/platform/graphics/dark_mode_settings.h"
-#include "third_party/blink/renderer/platform/graphics/graphics_types.h"
-#include "third_party/blink/renderer/platform/graphics/paint/paint_image.h"
+#include "third_party/blink/renderer/platform/graphics/dark_mode_types.h"
 #include "third_party/blink/renderer/platform/platform_export.h"
-#include "third_party/skia/include/core/SkRefCnt.h"
 
 class SkColorFilter;
 
@@ -48,19 +45,31 @@ class PLATFORM_EXPORT DarkModeFilter {
       const cc::PaintFlags& flags,
       ElementRole element_role);
 
-  // Decides whether to apply dark mode or not based on |src| and |dst|. True
-  // means dark mode should be applied. For applying the dark mode color filter
-  // to the image call ApplyToImageFlagsIfNeeded().
-  bool AnalyzeShouldApplyToImage(const SkRect& src, const SkRect& dst);
+  // Decides whether to apply dark mode or not based on |src| and |dst|.
+  // DarkModeResult::kDoNotApplyFilter - Dark mode filter should not be applied.
+  // DarkModeResult::kApplyFilter - Dark mode filter should be applied and to
+  // get the color filter GetImageFilter() should be called.
+  // DarkModeResult::kNotClassified - Dark mode filter should be applied and to
+  // get the color filter ApplyToImage() should be called.
+  DarkModeResult AnalyzeShouldApplyToImage(const SkRect& src,
+                                           const SkRect& dst) const;
+
+  // Returns dark mode color filter based on the classification done on
+  // |pixmap|. The image cannot be classified if pixmap is empty or |src| is
+  // empty or |src| is larger than pixmap bounds. Before calling this function
+  // AnalyzeShouldApplyToImage() must be called for early out or deciding
+  // appropriate function call. This function should be called only if image
+  // policy is set to DarkModeImagePolicy::kFilterSmart.
+  sk_sp<SkColorFilter> ApplyToImage(const SkPixmap& pixmap,
+                                    const SkRect& src,
+                                    const SkRect& dst);
+
+  // Returns dark mode color filter for images. Before calling this function
+  // AnalyzeShouldApplyToImage() must be called for early out or deciding
+  // appropriate function call. This function should be called only if image
+  // policy is set to DarkModeImagePolicy::kFilterAll.
+  sk_sp<SkColorFilter> GetImageFilter();
 
-  // Sets dark mode color filter on the flags based on the classification done
-  // on |paint_image|. |flags| must not be null.
-  void ApplyToImageFlagsIfNeeded(const SkRect& src,
-                                 const SkRect& dst,
-                                 const PaintImage& paint_image,
-                                 cc::PaintFlags* flags);
-
-  SkColorFilter* GetImageFilterForTesting() { return image_filter_.get(); }
   size_t GetInvertedColorCacheSizeForTesting();
 
  private:
@@ -69,10 +78,6 @@ class PLATFORM_EXPORT DarkModeFilter {
   DarkModeSettings settings_;
 
   bool ShouldApplyToColor(SkColor color, ElementRole role);
-  DarkModeClassification ClassifyImage(const DarkModeSettings& settings,
-                                       const SkRect& src,
-                                       const SkRect& dst,
-                                       const PaintImage& paint_image);
 
   std::unique_ptr<DarkModeColorClassifier> text_classifier_;
   std::unique_ptr<DarkModeColorClassifier> background_classifier_;

third_party/blink/renderer/platform/graphics/dark_mode_filter_helper.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 "third_party/blink/renderer/platform/graphics/dark_mode_filter_helper.h"
+
+#include "base/hash/hash.h"
+#include "third_party/blink/renderer/platform/graphics/dark_mode_filter.h"
+#include "third_party/blink/renderer/platform/graphics/dark_mode_image_cache.h"
+#include "third_party/blink/renderer/platform/graphics/image.h"
+
+namespace blink {
+
+// static
+void DarkModeFilterHelper::ApplyToImageIfNeeded(
+    DarkModeFilter* dark_mode_filter,
+    Image* image,
+    cc::PaintFlags* flags,
+    const SkRect& src,
+    const SkRect& dst) {
+  DCHECK(dark_mode_filter);
+  DCHECK(image);
+  DCHECK(flags);
+
+  // The Image::AsSkBitmapForCurrentFrame() is expensive due creation of paint
+  // image and bitmap, so ensure IsDarkModeActive() is checked prior to calling
+  // this function. See: https://crbug.com/1094781.
+  DCHECK(dark_mode_filter->IsDarkModeActive());
+
+  sk_sp<SkColorFilter> filter;
+  DarkModeResult result = dark_mode_filter->AnalyzeShouldApplyToImage(src, dst);
+
+  if (result == DarkModeResult::kApplyFilter) {
+    filter = dark_mode_filter->GetImageFilter();
+  } else if (result == DarkModeResult::kNotClassified) {
+    DarkModeImageCache* cache = image->GetDarkModeImageCache();
+    DCHECK(cache);
+    if (cache->Exists(src)) {
+      filter = cache->Get(src);
+    } else {
+      // Performance warning: Calling this function will synchronously decode
+      // image.
+      SkBitmap bitmap =
+          image->AsSkBitmapForCurrentFrame(kDoNotRespectImageOrientation);
+      SkPixmap pixmap;
+      bitmap.peekPixels(&pixmap);
+      filter = dark_mode_filter->ApplyToImage(pixmap, src, dst);
+      cache->Add(src, filter);
+    }
+  }
+
+  if (filter)
+    flags->setColorFilter(filter);
+}
+
+}  // namespace blink

third_party/blink/renderer/platform/graphics/dark_mode_filter_helper.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 THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_DARK_MODE_FILTER_HELPER_H_
+#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_DARK_MODE_FILTER_HELPER_H_
+
+#include "third_party/blink/renderer/platform/graphics/paint/paint_image.h"
+#include "third_party/blink/renderer/platform/platform_export.h"
+#include "third_party/skia/include/core/SkRect.h"
+
+namespace blink {
+
+class DarkModeFilter;
+class Image;
+
+class PLATFORM_EXPORT DarkModeFilterHelper {
+ public:
+  static void ApplyToImageIfNeeded(DarkModeFilter* dark_mode_filter,
+                                   Image* image,
+                                   cc::PaintFlags* flags,
+                                   const SkRect& src,
+                                   const SkRect& dst);
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_DARK_MODE_FILTER_HELPER_H_

third_party/blink/renderer/platform/graphics/dark_mode_filter_test.cc

@@ -30,8 +30,6 @@ TEST(DarkModeFilterTest, DoNotApplyFilterWhenDarkModeIsOff) {
   EXPECT_EQ(base::nullopt,
             filter.ApplyToFlagsIfNeeded(
                 cc::PaintFlags(), DarkModeFilter::ElementRole::kBackground));
-
-  EXPECT_EQ(nullptr, filter.GetImageFilterForTesting());
 }
 
 TEST(DarkModeFilterTest, ApplyDarkModeToColorsAndFlags) {
@@ -61,8 +59,6 @@ TEST(DarkModeFilterTest, ApplyDarkModeToColorsAndFlags) {
       flags, DarkModeFilter::ElementRole::kBackground);
   ASSERT_NE(flags_or_nullopt, base::nullopt);
   EXPECT_EQ(SK_ColorBLACK, flags_or_nullopt.value().getColor());
-
-  EXPECT_NE(nullptr, filter.GetImageFilterForTesting());
 }
 
 TEST(DarkModeFilterTest, InvertedColorCacheSize) {
@@ -122,32 +118,39 @@ TEST(DarkModeFilterTest, AnalyzeShouldApplyToImage) {
   filter.UpdateSettings(settings);
 
   // |dst| is smaller than threshold size.
-  EXPECT_TRUE(filter.AnalyzeShouldApplyToImage(SkRect::MakeWH(100, 100),
-                                               SkRect::MakeWH(100, 100)));
+  EXPECT_EQ(filter.AnalyzeShouldApplyToImage(SkRect::MakeWH(100, 100),
+                                             SkRect::MakeWH(100, 100)),
+            DarkModeResult::kNotClassified);
 
   // |dst| is smaller than threshold size, even |src| is larger.
-  EXPECT_TRUE(filter.AnalyzeShouldApplyToImage(SkRect::MakeWH(200, 200),
-                                               SkRect::MakeWH(100, 100)));
+  EXPECT_EQ(filter.AnalyzeShouldApplyToImage(SkRect::MakeWH(200, 200),
+                                             SkRect::MakeWH(100, 100)),
+            DarkModeResult::kNotClassified);
 
   // |dst| is smaller than threshold size, |src| is smaller.
-  EXPECT_TRUE(filter.AnalyzeShouldApplyToImage(SkRect::MakeWH(20, 20),
-                                               SkRect::MakeWH(100, 100)));
+  EXPECT_EQ(filter.AnalyzeShouldApplyToImage(SkRect::MakeWH(20, 20),
+                                             SkRect::MakeWH(100, 100)),
+            DarkModeResult::kNotClassified);
 
   // |src| having very smaller width, even |dst| is larger than threshold size.
-  EXPECT_TRUE(filter.AnalyzeShouldApplyToImage(SkRect::MakeWH(5, 200),
-                                               SkRect::MakeWH(5, 200)));
+  EXPECT_EQ(filter.AnalyzeShouldApplyToImage(SkRect::MakeWH(5, 200),
+                                             SkRect::MakeWH(5, 200)),
+            DarkModeResult::kNotClassified);
 
   // |src| having very smaller height, even |dst| is larger than threshold size.
-  EXPECT_TRUE(filter.AnalyzeShouldApplyToImage(SkRect::MakeWH(200, 5),
-                                               SkRect::MakeWH(200, 5)));
+  EXPECT_EQ(filter.AnalyzeShouldApplyToImage(SkRect::MakeWH(200, 5),
+                                             SkRect::MakeWH(200, 5)),
+            DarkModeResult::kNotClassified);
 
   // |dst| is larger than threshold size.
-  EXPECT_FALSE(filter.AnalyzeShouldApplyToImage(SkRect::MakeWH(20, 20),
-                                                SkRect::MakeWH(200, 200)));
+  EXPECT_EQ(filter.AnalyzeShouldApplyToImage(SkRect::MakeWH(20, 20),
+                                             SkRect::MakeWH(200, 200)),
+            DarkModeResult::kDoNotApplyFilter);
 
   // |dst| is larger than threshold size.
-  EXPECT_FALSE(filter.AnalyzeShouldApplyToImage(SkRect::MakeWH(20, 200),
-                                                SkRect::MakeWH(20, 200)));
+  EXPECT_EQ(filter.AnalyzeShouldApplyToImage(SkRect::MakeWH(20, 200),
+                                             SkRect::MakeWH(20, 200)),
+            DarkModeResult::kDoNotApplyFilter);
 }
 
 }  // namespace

third_party/blink/renderer/platform/graphics/dark_mode_image_cache.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 THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_DARK_MODE_IMAGE_CACHE_H_
+#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_DARK_MODE_IMAGE_CACHE_H_
+
+#include <unordered_map>
+
+#include "base/hash/hash.h"
+#include "third_party/blink/renderer/platform/platform_export.h"
+#include "third_party/skia/include/core/SkColorFilter.h"
+
+namespace blink {
+
+// DarkModeImageCache - Implements dark mode filter cache for different |src|
+// rects from the image.
+class PLATFORM_EXPORT DarkModeImageCache {
+ public:
+  DarkModeImageCache() = default;
+  ~DarkModeImageCache() = default;
+
+  bool Exists(const SkRect& src) {
+    return cache_.find(DarkModeKey(src)) != cache_.end();
+  }
+
+  sk_sp<SkColorFilter> Get(const SkRect& src) {
+    auto result = cache_.find(DarkModeKey(src));
+    return (result != cache_.end()) ? result->second : nullptr;
+  }
+
+  void Add(const SkRect& src, sk_sp<SkColorFilter> dark_mode_color_filter) {
+    DCHECK(!Exists(src));
+
+    cache_.emplace(DarkModeKey(src), std::move(dark_mode_color_filter));
+  }
+
+  size_t Size() { return cache_.size(); }
+
+  void Clear() { cache_.clear(); }
+
+ private:
+  struct DarkModeKeyHash;
+  struct DarkModeKey {
+    explicit DarkModeKey(SkRect src) : src_(src) {}
+
+    bool operator==(const DarkModeKey& other) const {
+      return src_ == other.src_;
+    }
+
+   private:
+    SkRect src_;
+
+    friend struct DarkModeImageCache::DarkModeKeyHash;
+  };
+
+  struct DarkModeKeyHash {
+    size_t operator()(const DarkModeKey& key) const {
+      return base::HashInts(
+          base::HashInts(base::HashInts(key.src_.x(), key.src_.y()),
+                         key.src_.width()),
+          key.src_.height());
+    }
+  };
+
+  std::unordered_map<DarkModeKey, sk_sp<SkColorFilter>, DarkModeKeyHash> cache_;
+
+  DISALLOW_COPY_AND_ASSIGN(DarkModeImageCache);
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_DARK_MODE_IMAGE_CACHE_H_

third_party/blink/renderer/platform/graphics/dark_mode_image_cache_test.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 "third_party/blink/renderer/platform/graphics/dark_mode_image_cache.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/effects/SkHighContrastFilter.h"
+
+namespace blink {
+
+class DarkModeImageCacheTest : public testing::Test {};
+
+TEST_F(DarkModeImageCacheTest, Caching) {
+  DarkModeImageCache cache;
+
+  SkHighContrastConfig config;
+  config.fInvertStyle = SkHighContrastConfig::InvertStyle::kInvertLightness;
+  sk_sp<SkColorFilter> filter = SkHighContrastFilter::Make(config);
+
+  SkRect src1 = SkRect::MakeXYWH(0, 0, 50, 50);
+  SkRect src2 = SkRect::MakeXYWH(5, 20, 100, 100);
+  SkRect src3 = SkRect::MakeXYWH(6, -9, 50, 50);
+
+  EXPECT_FALSE(cache.Exists(src1));
+  EXPECT_EQ(cache.Get(src1), nullptr);
+  cache.Add(src1, filter);
+  EXPECT_TRUE(cache.Exists(src1));
+  EXPECT_EQ(cache.Get(src1), filter);
+
+  EXPECT_FALSE(cache.Exists(src2));
+  EXPECT_EQ(cache.Get(src2), nullptr);
+  cache.Add(src2, nullptr);
+  EXPECT_TRUE(cache.Exists(src2));
+  EXPECT_EQ(cache.Get(src2), nullptr);
+
+  EXPECT_EQ(cache.Size(), 2u);
+  cache.Clear();
+  EXPECT_EQ(cache.Size(), 0u);
+
+  EXPECT_FALSE(cache.Exists(src1));
+  EXPECT_EQ(cache.Get(src1), nullptr);
+  EXPECT_FALSE(cache.Exists(src2));
+  EXPECT_EQ(cache.Get(src2), nullptr);
+  EXPECT_FALSE(cache.Exists(src3));
+  EXPECT_EQ(cache.Get(src3), nullptr);
+  cache.Add(src3, filter);
+  EXPECT_TRUE(cache.Exists(src3));
+  EXPECT_EQ(cache.Get(src3), filter);
+
+  EXPECT_EQ(cache.Size(), 1u);
+}
+
+}  // namespace blink

third_party/blink/renderer/platform/graphics/dark_mode_image_classifier.h

@@ -9,17 +9,15 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/optional.h"
-#include "third_party/blink/renderer/platform/graphics/graphics_types.h"
-#include "third_party/blink/renderer/platform/graphics/paint/paint_image.h"
-#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
-#include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/blink/renderer/platform/graphics/dark_mode_types.h"
+#include "third_party/blink/renderer/platform/platform_export.h"
+#include "third_party/skia/include/core/SkPixmap.h"
 #include "third_party/skia/include/core/SkRect.h"
-#include "ui/gfx/geometry/rect.h"
 
 namespace blink {
 
+FORWARD_DECLARE_TEST(DarkModeImageClassifierTest, BlockSamples);
 FORWARD_DECLARE_TEST(DarkModeImageClassifierTest, FeaturesAndClassification);
-FORWARD_DECLARE_TEST(DarkModeImageClassifierTest, Caching);
 
 // This class is not threadsafe as the cache used for storing classification
 // results is not threadsafe. So it can be used only in blink main thread.
@@ -43,38 +41,27 @@ class PLATFORM_EXPORT DarkModeImageClassifier {
     float background_ratio;
   };
 
-  // Performance warning: |paint_image| will be synchronously decoded if this
-  // function is called in blink main thread.
-  DarkModeClassification Classify(const PaintImage& paint_image,
-                                  const SkRect& src,
-                                  const SkRect& dst);
-
-  // Removes cache identified by given |image_id|.
-  static void RemoveCache(PaintImage::Id image_id);
+  DarkModeResult Classify(const SkPixmap& pixmap, const SkRect& src);
 
  private:
-  DarkModeClassification ClassifyWithFeatures(const Features& features);
-  DarkModeClassification ClassifyUsingDecisionTree(const Features& features);
-  bool GetBitmap(const PaintImage& paint_image,
-                 const SkRect& src,
-                 SkBitmap* bitmap);
-  base::Optional<Features> GetFeatures(const PaintImage& paint_image,
-                                       const SkRect& src);
+  DarkModeResult ClassifyWithFeatures(const Features& features);
+  DarkModeResult ClassifyUsingDecisionTree(const Features& features);
 
   enum class ColorMode { kColor = 0, kGrayscale = 1 };
 
+  base::Optional<Features> GetFeatures(const SkPixmap& pixmap,
+                                       const SkRect& src);
   // Extracts a sample set of pixels (|sampled_pixels|), |transparency_ratio|,
   // and |background_ratio|.
-  void GetSamples(const PaintImage& paint_image,
+  void GetSamples(const SkPixmap& pixmap,
                   const SkRect& src,
                   std::vector<SkColor>* sampled_pixels,
                   float* transparency_ratio,
                   float* background_ratio);
-
   // Gets the |required_samples_count| for a specific |block| of the given
-  // SkBitmap, and returns |sampled_pixels| and |transparent_pixels_count|.
-  void GetBlockSamples(const SkBitmap& bitmap,
-                       const gfx::Rect& block,
+  // pixmap, and returns |sampled_pixels| and |transparent_pixels_count|.
+  void GetBlockSamples(const SkPixmap& pixmap,
+                       const SkIRect& block,
                        const int required_samples_count,
                        std::vector<SkColor>* sampled_pixels,
                        int* transparent_pixels_count);
@@ -92,19 +79,9 @@ class PLATFORM_EXPORT DarkModeImageClassifier {
   float ComputeColorBucketsRatio(const std::vector<SkColor>& sampled_pixels,
                                  const ColorMode color_mode);
 
-  // Gets cached value from the given |image_id| cache.
-  DarkModeClassification GetCacheValue(PaintImage::Id image_id,
-                                       const SkRect& src);
-  // Adds cache value |result| to the given |image_id| cache.
-  void AddCacheValue(PaintImage::Id image_id,
-                     const SkRect& src,
-                     DarkModeClassification result);
-  // Returns the cache size for the given |image_id|.
-  size_t GetCacheSize(PaintImage::Id image_id);
-
+  FRIEND_TEST_ALL_PREFIXES(DarkModeImageClassifierTest, BlockSamples);
   FRIEND_TEST_ALL_PREFIXES(DarkModeImageClassifierTest,
                            FeaturesAndClassification);
-  FRIEND_TEST_ALL_PREFIXES(DarkModeImageClassifierTest, Caching);
 };
 
 }  // namespace blink

third_party/blink/renderer/platform/graphics/dark_mode_types.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 THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_DARK_MODE_TYPES_H_
+#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_DARK_MODE_TYPES_H_
+
+namespace blink {
+
+enum class DarkModeResult : uint8_t {
+  kDoNotApplyFilter,
+  kApplyFilter,
+  kNotClassified
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_DARK_MODE_TYPES_H_

third_party/blink/renderer/platform/graphics/graphics_context.cc

@@ -37,6 +37,7 @@
 #include "third_party/blink/renderer/platform/geometry/float_rounded_rect.h"
 #include "third_party/blink/renderer/platform/geometry/int_rect.h"
 #include "third_party/blink/renderer/platform/graphics/dark_mode_filter.h"
+#include "third_party/blink/renderer/platform/graphics/dark_mode_filter_helper.h"
 #include "third_party/blink/renderer/platform/graphics/graphics_context_state_saver.h"
 #include "third_party/blink/renderer/platform/graphics/interpolation_space.h"
 #include "third_party/blink/renderer/platform/graphics/paint/paint_canvas.h"
@@ -877,10 +878,9 @@ void GraphicsContext::DrawImage(
   image_flags.setFilterQuality(ComputeFilterQuality(image, dest, src));
 
   // Do not classify the image if the element has any CSS filters.
-  if (!has_filter_property && dark_mode_filter_.IsDarkModeActive() &&
-      dark_mode_filter_.AnalyzeShouldApplyToImage(src, dest)) {
-    dark_mode_filter_.ApplyToImageFlagsIfNeeded(
-        src, dest, image->PaintImageForCurrentFrame(), &image_flags);
+  if (!has_filter_property && dark_mode_filter_.IsDarkModeActive()) {
+    DarkModeFilterHelper::ApplyToImageIfNeeded(&dark_mode_filter_, image,
+                                               &image_flags, src, dest);
   }
 
   image->Draw(canvas_, image_flags, dest, src, should_respect_image_orientation,
@@ -918,11 +918,9 @@ void GraphicsContext::DrawImageRRect(
   image_flags.setFilterQuality(
       ComputeFilterQuality(image, dest.Rect(), src_rect));
 
-  if (dark_mode_filter_.IsDarkModeActive() &&
-      dark_mode_filter_.AnalyzeShouldApplyToImage(src_rect, dest.Rect())) {
-    dark_mode_filter_.ApplyToImageFlagsIfNeeded(
-        src_rect, dest.Rect(), image->PaintImageForCurrentFrame(),
-        &image_flags);
+  if (dark_mode_filter_.IsDarkModeActive()) {
+    DarkModeFilterHelper::ApplyToImageIfNeeded(
+        &dark_mode_filter_, image, &image_flags, src_rect, dest.Rect());
   }
 
   bool use_shader = (visible_src == src_rect) &&

third_party/blink/renderer/platform/graphics/graphics_types.h

@@ -134,12 +134,6 @@ enum MailboxSyncMode {
   kOrderingBarrier,
 };
 
-enum class DarkModeClassification {
-  kNotClassified,
-  kApplyFilter,
-  kDoNotApplyFilter,
-};
-
 // TODO(junov): crbug.com/453113 Relocate ShadowMode to
 // CanvasRenderingContext2DState.h once GraphicsContext no longer uses it.
 enum ShadowMode {

third_party/blink/renderer/platform/graphics/image.cc

@@ -40,6 +40,7 @@
 #include "third_party/blink/renderer/platform/geometry/float_size.h"
 #include "third_party/blink/renderer/platform/geometry/length.h"
 #include "third_party/blink/renderer/platform/graphics/bitmap_image.h"
+#include "third_party/blink/renderer/platform/graphics/dark_mode_image_cache.h"
 #include "third_party/blink/renderer/platform/graphics/dark_mode_image_classifier.h"
 #include "third_party/blink/renderer/platform/graphics/deferred_image_decoder.h"
 #include "third_party/blink/renderer/platform/graphics/graphics_context.h"
@@ -65,12 +66,7 @@ Image::Image(ImageObserver* observer, bool is_multipart)
       stable_image_id_(PaintImage::GetNextId()),
       is_multipart_(is_multipart) {}
 
-Image::~Image() {
-  // TODO(prashant.n): This logic is needed to purge cache for the same origin
-  // page navigations. Redesign this once dark mode filter module gets moved to
-  // compositor side.
-  DarkModeImageClassifier::RemoveCache(stable_image_id_);
-}
+Image::~Image() = default;
 
 Image* Image::NullImage() {
   DCHECK(IsMainThread());
@@ -365,6 +361,13 @@ SkBitmap Image::AsSkBitmapForCurrentFrame(
   return bitmap;
 }
 
+DarkModeImageCache* Image::GetDarkModeImageCache() {
+  if (!dark_mode_image_cache_)
+    dark_mode_image_cache_ = std::make_unique<DarkModeImageCache>();
+
+  return dark_mode_image_cache_.get();
+}
+
 FloatRect Image::CorrectSrcRectForImageOrientation(FloatSize image_size,
                                                    FloatRect src_rect) const {
   ImageOrientation orientation = CurrentFrameOrientation();

third_party/blink/renderer/platform/graphics/image.h

@@ -61,6 +61,7 @@ class GraphicsContext;
 class Image;
 class WebGraphicsContext3DProvider;
 class WebGraphicsContext3DProviderWrapper;
+class DarkModeImageCache;
 
 class PLATFORM_EXPORT Image : public ThreadSafeRefCounted<Image> {
   friend class GeneratedImage;
@@ -252,6 +253,8 @@ class PLATFORM_EXPORT Image : public ThreadSafeRefCounted<Image> {
   // Returns an SkBitmap that is a copy of the image's current frame.
   SkBitmap AsSkBitmapForCurrentFrame(RespectImageOrientationEnum);
 
+  DarkModeImageCache* GetDarkModeImageCache();
+
  protected:
   Image(ImageObserver* = nullptr, bool is_multipart = false);
 
@@ -284,6 +287,7 @@ class PLATFORM_EXPORT Image : public ThreadSafeRefCounted<Image> {
   WeakPersistent<ImageObserver> image_observer_;
   PaintImage::Id stable_image_id_;
   const bool is_multipart_;
+  std::unique_ptr<DarkModeImageCache> dark_mode_image_cache_;
   DISALLOW_COPY_AND_ASSIGN(Image);
 };