Commit 332fc255 authored by prashant.n's avatar prashant.n Committed by Commit Bot

Remove DarkModePagePolicy.

The dark mode page policy is used for deciding whether to apply dark
mode filter to the given LayoutView or not based on values kFilterAll
(always process) or kFilterByBackground (process based on background
color.) The current code has kFilterByBackground as a default value.
But processing based on background of root layout object can lead
issues like not applying dark mode to children having light or
transparent backgrounds.

This patch removes page policy concept and now every layout object
gets classified for applying dark mode. All the sites for which page
policy kFilterByBackground was added, work properly after removing
the page policy concept.

This now helps pages from e.g. theverge.com render correctly in dark
mode. In these pages, children of dark backgrounds get dark mode
applied correctly and look dark. Previously they were getting rendered
as if dark mode was not applied.

Bug: 1081383
Change-Id: I1e6a941bad22689569f2aec76d5beb88daced9de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2323441Reviewed-by: default avatarRune Lillesveen <futhark@chromium.org>
Reviewed-by: default avatarAnna Malova <amalova@chromium.org>
Reviewed-by: default avatarStephen Chenney <schenney@chromium.org>
Commit-Queue: Prashant Nevase <prashant.n@samsung.com>
Cr-Commit-Position: refs/heads/master@{#800573}
parent 914686c5
......@@ -25,7 +25,6 @@ blink_core_sources("accessibility") {
blink_core_tests("unit_tests") {
sources = [
"apply_dark_mode_test.cc",
"blink_ax_event_intent_test.cc",
"scoped_blink_ax_event_intent_test.cc",
]
......
......@@ -16,29 +16,6 @@
namespace blink {
namespace {
const int kAlphaThreshold = 100;
const int kBrightnessThreshold = 50;
// TODO(https://crbug.com/925949): Add detection and classification of
// background image color. Most sites with dark background images also have a
// dark background color set, so this is less of a priority than it would be
// otherwise.
bool HasLightBackground(const LayoutView& root) {
const ComputedStyle& style = root.StyleRef();
// If we can't easily determine the background color, default to inverting the
// page.
if (!style.HasBackground())
return true;
Color color = style.VisitedDependentColor(GetCSSPropertyBackgroundColor());
if (color.Alpha() < kAlphaThreshold)
return true;
return DarkModeColorClassifier::CalculateColorBrightness(color.Rgb()) >
kBrightnessThreshold;
}
DarkModeInversionAlgorithm GetMode(const Settings& frame_settings) {
switch (features::kForceDarkInversionMethodParam.Get()) {
case ForceDarkInversionMethod::kUseBlinkSettings:
......@@ -106,26 +83,13 @@ DarkModeSettings GetDisabledSettings() {
} // namespace
DarkModeSettings BuildDarkModeSettings(const Settings& frame_settings,
const LayoutView& root) {
if (frame_settings.GetForceDarkModeEnabled() &&
ShouldApplyDarkModeFilterToPage(
frame_settings.GetForceDarkModePagePolicy(), root)) {
return GetEnabledSettings(frame_settings);
}
return GetDisabledSettings();
}
bool content_has_dark_color_scheme) {
if (content_has_dark_color_scheme)
return GetDisabledSettings();
bool ShouldApplyDarkModeFilterToPage(DarkModePagePolicy policy,
const LayoutView& root) {
if (root.StyleRef().DarkColorScheme())
return false;
switch (policy) {
case DarkModePagePolicy::kFilterAll:
return true;
case DarkModePagePolicy::kFilterByBackground:
return HasLightBackground(root);
}
return frame_settings.GetForceDarkModeEnabled()
? GetEnabledSettings(frame_settings)
: GetDisabledSettings();
}
} // namespace blink
......@@ -7,22 +7,15 @@
#include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/core/frame/settings.h"
#include "third_party/blink/renderer/core/layout/layout_view.h"
#include "third_party/blink/renderer/platform/graphics/dark_mode_settings.h"
namespace blink {
// Extract dark mode settings from |settings| and modify them as needed
// based on |root|.
DarkModeSettings CORE_EXPORT BuildDarkModeSettings(const Settings& settings,
const LayoutView& root);
// Determine whether the page with the provided |root| should have its colors
// inverted, based on the provided |policy|.
//
// This method does not check whether Dark Mode is enabled overall.
bool CORE_EXPORT ShouldApplyDarkModeFilterToPage(DarkModePagePolicy policy,
const LayoutView& root);
// If content has dark color scheme set then return disabled settings, otherwise
// return enabled settings based on force dark mode enabled.
DarkModeSettings CORE_EXPORT
BuildDarkModeSettings(const Settings& settings,
bool content_has_dark_color_scheme);
} // namespace blink
......
// Copyright 2019 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/core/accessibility/apply_dark_mode.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/renderer/core/html/html_head_element.h"
#include "third_party/blink/renderer/core/style/computed_style.h"
#include "third_party/blink/renderer/core/testing/color_scheme_helper.h"
#include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
namespace blink {
namespace {
using ApplyDarkModeCheckTest = RenderingTest;
TEST_F(ApplyDarkModeCheckTest, LightSolidBackgroundAlwaysFiltered) {
GetDocument().body()->SetInlineStyleProperty(CSSPropertyID::kBackgroundColor,
CSSValueID::kWhite);
UpdateAllLifecyclePhasesForTest();
EXPECT_TRUE(ShouldApplyDarkModeFilterToPage(
DarkModePagePolicy::kFilterByBackground, GetLayoutView()));
EXPECT_TRUE(ShouldApplyDarkModeFilterToPage(DarkModePagePolicy::kFilterAll,
GetLayoutView()));
}
TEST_F(ApplyDarkModeCheckTest, DarkSolidBackgroundFilteredIfPolicyIsFilterAll) {
GetDocument().body()->SetInlineStyleProperty(CSSPropertyID::kBackgroundColor,
CSSValueID::kBlack);
UpdateAllLifecyclePhasesForTest();
EXPECT_FALSE(ShouldApplyDarkModeFilterToPage(
DarkModePagePolicy::kFilterByBackground, GetLayoutView()));
EXPECT_TRUE(ShouldApplyDarkModeFilterToPage(DarkModePagePolicy::kFilterAll,
GetLayoutView()));
}
TEST_F(ApplyDarkModeCheckTest, DarkTransparentBackgroundAlwaysFiltered) {
GetDocument().body()->SetInlineStyleProperty(CSSPropertyID::kBackgroundColor,
CSSValueID::kTransparent);
UpdateAllLifecyclePhasesForTest();
EXPECT_TRUE(ShouldApplyDarkModeFilterToPage(
DarkModePagePolicy::kFilterByBackground, GetLayoutView()));
EXPECT_TRUE(ShouldApplyDarkModeFilterToPage(DarkModePagePolicy::kFilterAll,
GetLayoutView()));
}
TEST_F(ApplyDarkModeCheckTest, BackgroundColorNotDefinedAlwaysFiltered) {
GetDocument().body()->RemoveInlineStyleProperty(
CSSPropertyID::kBackgroundColor);
UpdateAllLifecyclePhasesForTest();
EXPECT_TRUE(ShouldApplyDarkModeFilterToPage(
DarkModePagePolicy::kFilterByBackground, GetLayoutView()));
EXPECT_TRUE(ShouldApplyDarkModeFilterToPage(DarkModePagePolicy::kFilterAll,
GetLayoutView()));
}
TEST_F(ApplyDarkModeCheckTest, MetaColorSchemeDark) {
ScopedCSSColorSchemeForTest css_feature_scope(true);
ScopedMetaColorSchemeForTest meta_feature_scope(true);
GetDocument().GetSettings()->SetForceDarkModeEnabled(true);
ColorSchemeHelper color_scheme_helper(GetDocument());
color_scheme_helper.SetPreferredColorScheme(PreferredColorScheme::kDark);
GetDocument().head()->setInnerHTML(R"HTML(
<meta name="color-scheme" content="dark">
)HTML");
UpdateAllLifecyclePhasesForTest();
// Opting out of forced darkening when dark is among the supported color
// schemes for the page.
EXPECT_FALSE(ShouldApplyDarkModeFilterToPage(
DarkModePagePolicy::kFilterByBackground, GetLayoutView()));
EXPECT_FALSE(ShouldApplyDarkModeFilterToPage(DarkModePagePolicy::kFilterAll,
GetLayoutView()));
}
} // namespace
} // namespace blink
......@@ -2934,8 +2934,8 @@ void LocalFrameView::PaintTree(
} else {
GraphicsContext graphics_context(*paint_controller_);
if (Settings* settings = frame_->GetSettings()) {
graphics_context.SetDarkMode(
BuildDarkModeSettings(*settings, *GetLayoutView()));
graphics_context.SetDarkMode(BuildDarkModeSettings(
*settings, GetLayoutView()->StyleRef().DarkColorScheme()));
}
bool painted_full_screen_overlay = false;
......
......@@ -902,12 +902,6 @@
type: "DarkModeImagePolicy",
invalidate: "ForceDark",
},
{
name: "forceDarkModePagePolicy",
initial: "DarkModePagePolicy::kFilterByBackground",
type: "DarkModePagePolicy",
invalidate: "ForceDark",
},
{
name: "forceDarkModeTextBrightnessThreshold",
initial: "150",
......
......@@ -1635,8 +1635,8 @@ void CompositedLayerMapping::DoPaintTask(
paint_info.paint_layer->GetLayoutObject().GetFrame());
context.SetDeviceScaleFactor(device_scale_factor);
Settings* settings = GetLayoutObject().GetFrame()->GetSettings();
context.SetDarkMode(
BuildDarkModeSettings(*settings, *GetLayoutObject().View()));
context.SetDarkMode(BuildDarkModeSettings(
*settings, GetLayoutObject().View()->StyleRef().DarkColorScheme()));
// As a composited layer may be painted directly, we need to traverse the
// effect tree starting from the current node all the way up through the
......
......@@ -28,13 +28,6 @@ enum class DarkModeImagePolicy {
kFilterSmart,
};
enum class DarkModePagePolicy {
// Apply dark-mode filter to all frames, regardless of content.
kFilterAll,
// Apply dark-mode filter to frames based on background color.
kFilterByBackground,
};
// New variables added to this struct should also be added to
// BuildDarkModeSettings() in
// //src/third_party/blink/renderer/core/accessibility/apply_dark_mode.h
......
......@@ -271,14 +271,9 @@
"args": ["--blink-settings=forceDarkModeEnabled=true,forceDarkModeInversionAlgorithm=3,forceDarkModeTextBrightnessThreshold=256,forceDarkModeBackgroundBrightnessThreshold=0"]
},
{
"prefix": "dark-mode-page-policy-all",
"prefix": "dark-mode-page-background",
"bases": [],
"args": ["--blink-settings=forceDarkModeEnabled=true,forceDarkModeInversionAlgorithm=3,forceDarkModePagePolicy=0,forceDarkModeTextBrightnessThreshold=256,forceDarkModeBackgroundBrightnessThreshold=0"]
},
{
"prefix": "dark-mode-page-policy-background",
"bases": [],
"args": ["--blink-settings=forceDarkModeEnabled=true,forceDarkModeInversionAlgorithm=3,forceDarkModePagePolicy=1,forceDarkModeTextBrightnessThreshold=256,forceDarkModeBackgroundBrightnessThreshold=0"]
"args": ["--blink-settings=forceDarkModeEnabled=true,forceDarkModeInversionAlgorithm=3,forceDarkModeTextBrightnessThreshold=256,forceDarkModeBackgroundBrightnessThreshold=0"]
},
{
"prefix": "dark-mode-text",
......
# This suite runs the tests in LayoutTests/paint/dark-mode
# with --blink-settings="darkMode=3,darkModePagePolicy=1"
# with --blink-settings="darkMode=3"
# See the virtual_test_suites() method in tools/blinkpy/web_tests/port/base.py.
# This suite runs the tests in LayoutTests/paint/dark-mode
# with --blink-settings="darkMode=3,darkModePagePolicy=0"
# See the virtual_test_suites() method in tools/blinkpy/web_tests/port/base.py.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment