0

[color pipeline] Add ColorProviderSource/Observer to WebContents

This CL updates WebContentsImpl to implement the
ColorProviderSourceObserver interface. SetColorProviderSource()
has been added to WebContents to allow clients to use the
public WebContents interface to set the appropriate source for
a given context.

ColorProviderSource::GetColorProviderKey() has been added to
allow observers to get the ColorProviderKey currently associated
with the source's ColorProvider instance. This is used by
WebContentsImpl to allow it to get the appropriate ColorProvider
instance when its source has been reset. This helps to address
situations where WebContents can be added and removed from
a UI hierarchy in rapid succession.

views::WebView has been updated to ensure the correct
ColorProviderSource is set on its wrapped WebContents.

Bug: 1251637
Change-Id: Iec687eb218767bda3e8569b399dd4590cca59eb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3183368
Reviewed-by: Keren Zhu <kerenzhu@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Elaine Chien <elainechien@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#926558}
This commit is contained in:
tom
2021-09-30 00:27:26 +00:00
committed by Chromium LUCI CQ
parent 93a7c365dd
commit 855c4bf3ae
14 changed files with 271 additions and 20 deletions

@ -83,6 +83,7 @@ include_rules = [
# Aura is analogous to Win32 or a Gtk, so it is allowed.
"+ui/aura",
"+ui/base",
"+ui/color",
"+ui/compositor",
"+ui/display",
"+ui/events",

@ -238,6 +238,8 @@ source_set("browser") {
"//ui/base/dragdrop/mojom",
"//ui/base/idle",
"//ui/base/ime/init",
"//ui/color",
"//ui/color:mixers",
"//ui/display",
"//ui/display/types",
"//ui/display/util:gpu_info_util",

@ -160,6 +160,7 @@
#include "ui/accessibility/ax_tree_combiner.h"
#include "ui/base/pointer/pointer_device.h"
#include "ui/base/window_open_disposition.h"
#include "ui/color/color_provider_manager.h"
#include "ui/display/screen.h"
#include "ui/events/base_event_utils.h"
#include "ui/gfx/animation/animation.h"
@ -499,6 +500,25 @@ int64_t AdjustRequestedWindowBounds(gfx::Rect* bounds, RenderFrameHost* host) {
return display.id();
}
// WebContents can exist independently of a ColorProviderSource and is still
// expected to be able to be inserted into a system gfx::NativeView hierarchy.
// Whilst most WebContents clients should be setting a ColorProviderSource we
// must accommodate for cases where this is not currently being done. For these
// cases fallback to a ColorProvider keyed to various NativeTheme bits.
ui::ColorProviderManager::ColorProviderKey GetWebDefaultColorProviderKey() {
const auto* native_theme = ui::NativeTheme::GetInstanceForWeb();
const auto color_scheme = native_theme->GetDefaultSystemColorScheme();
return {(color_scheme == ui::NativeTheme::ColorScheme::kDark)
? ui::ColorProviderManager::ColorMode::kDark
: ui::ColorProviderManager::ColorMode::kLight,
(color_scheme == ui::NativeTheme::ColorScheme::kPlatformHighContrast)
? ui::ColorProviderManager::ContrastMode::kHigh
: ui::ColorProviderManager::ContrastMode::kNormal,
native_theme->is_custom_system_theme()
? ui::ColorProviderManager::SystemTheme::kCustom
: ui::ColorProviderManager::SystemTheme::kDefault};
}
} // namespace
CreatedWindow::CreatedWindow() = default;
@ -885,6 +905,7 @@ WebContentsImpl::WebContentsImpl(BrowserContext* browser_context)
std::make_unique<MediaWebContentsObserver>(this)),
is_overlay_content_(false),
showing_context_menu_(false),
color_provider_key_(GetWebDefaultColorProviderKey()),
prerender_host_registry_(blink::features::IsPrerender2Enabled()
? std::make_unique<PrerenderHostRegistry>()
: nullptr),
@ -1511,6 +1532,10 @@ void WebContentsImpl::SetPageBaseBackgroundColor(
page_base_background_color_));
}
void WebContentsImpl::SetColorProviderSource(ui::ColorProviderSource* source) {
ColorProviderSourceObserver::Observe(source);
}
void WebContentsImpl::SetAccessibilityMode(ui::AXMode mode) {
OPTIONAL_TRACE_EVENT2("content", "WebContentsImpl::SetAccessibilityMode",
"mode", mode.ToString(), "previous_mode",
@ -8848,6 +8873,22 @@ void WebContentsImpl::OnCaptionStyleUpdated() {
NotifyPreferencesChanged();
}
void WebContentsImpl::OnColorProviderChanged() {
// TODO(tluk): Code that needs to be notified of theme color changes (not just
// NativeTheme changes) should be moved here.
DCHECK(GetColorProviderSource());
color_provider_key_ = GetColorProviderSource()->GetColorProviderKey();
}
const ui::ColorProvider* WebContentsImpl::GetColorProvider() const {
// Always defer to the ColorProviderSource if available for the correct
// ColorProvider instance.
return GetColorProviderSource()
? GetColorProviderSource()->GetColorProvider()
: ui::ColorProviderManager::Get().GetColorProviderFor(
color_provider_key_);
}
blink::mojom::FrameWidgetInputHandler*
WebContentsImpl::GetFocusedFrameWidgetInputHandler() {
auto* focused_render_widget_host =

@ -72,6 +72,7 @@
#include "ui/accessibility/ax_mode.h"
#include "ui/accessibility/platform/inspect/ax_event_recorder.h"
#include "ui/base/dragdrop/mojom/drag_drop_types.mojom-forward.h"
#include "ui/color/color_provider_source_observer.h"
#include "ui/gfx/geometry/size.h"
#include "ui/native_theme/native_theme.h"
#include "ui/native_theme/native_theme_observer.h"
@ -182,7 +183,8 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
public blink::mojom::ColorChooserFactory,
public NavigationControllerDelegate,
public NavigatorDelegate,
public ui::NativeThemeObserver {
public ui::NativeThemeObserver,
public ui::ColorProviderSourceObserver {
public:
class FriendWrapper;
@ -354,6 +356,7 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
absl::optional<SkColor> GetThemeColor() override;
absl::optional<SkColor> GetBackgroundColor() override;
void SetPageBaseBackgroundColor(absl::optional<SkColor> color) override;
void SetColorProviderSource(ui::ColorProviderSource* source) override;
WebUI* GetWebUI() override;
void SetUserAgentOverride(const blink::UserAgentOverride& ua_override,
bool override_in_new_tabs) override;
@ -1724,6 +1727,13 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
void OnNativeThemeUpdated(ui::NativeTheme* observed_theme) override;
void OnCaptionStyleUpdated() override;
// ui::ColorProviderSourceObserver:
void OnColorProviderChanged() override;
// Returns the ColorProvider instance for this WebContents object. This will
// always return a valid ColorProvider instance.
const ui::ColorProvider* GetColorProvider() const;
// Sets the visibility to |new_visibility| and propagates this to the
// renderer side, taking into account the current capture state. This
// can be called with the current visibility to effect capturing
@ -2174,6 +2184,17 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
ui::NativeTheme::PreferredContrast preferred_contrast_ =
ui::NativeTheme::PreferredContrast::kNoPreference;
// A cached copy of the most recently observed ColorProviderKey. This is used
// as a fallback to get the most recently seen ColorProvider in situations
// where the ColorProviderSource is no longer available but the WebContents
// continues to require colors for rendering. This ensures the WebContents
// does not repaint with incorrect colors when removed from a UI hierarchy
// with a ColorProviderSource only to later be re-inserted into the same UI
// hierarchy. This pattern is present in the behavior of browser tabs where
// WebContents objects are added to and removed from the browser UI as the
// active tab changes.
ui::ColorProviderManager::ColorProviderKey color_provider_key_;
// Tracks clients who want to be notified when a JavaScript dialog is
// dismissed.
std::unique_ptr<JavaScriptDialogDismissNotifier>

@ -79,6 +79,7 @@ class InterfaceProvider;
namespace ui {
struct AXPropertyFilter;
struct AXTreeUpdate;
class ColorProviderSource;
}
namespace content {
@ -487,6 +488,12 @@ class WebContents : public PageNavigator,
// understand.
virtual void SetPageBaseBackgroundColor(absl::optional<SkColor> color) = 0;
// Sets the ColorProviderSource for the WebContents. The WebContents will
// maintain an observation of `source` until a new source is set or the
// current source is destroyed. WebContents will receive updates when the
// source's ColorProvider changes.
virtual void SetColorProviderSource(ui::ColorProviderSource* source) = 0;
// Returns the committed WebUI if one exists.
virtual WebUI* GetWebUI() = 0;

@ -74,6 +74,7 @@ test("color_unittests") {
sources = [
"color_mixer_unittest.cc",
"color_provider_manager_unittest.cc",
"color_provider_source_observer_unittest.cc",
"color_provider_unittest.cc",
"color_recipe_unittest.cc",
"color_transform_unittest.cc",

@ -16,7 +16,7 @@ ColorProviderSource::~ColorProviderSource() {
void ColorProviderSource::AddObserver(ColorProviderSourceObserver* observer) {
observers_.AddObserver(observer);
observer->OnColorProviderChanged(this);
observer->OnColorProviderChanged();
}
void ColorProviderSource::RemoveObserver(
@ -26,7 +26,7 @@ void ColorProviderSource::RemoveObserver(
void ColorProviderSource::NotifyColorProviderChanged() {
for (auto& observer : observers_)
observer.OnColorProviderChanged(this);
observer.OnColorProviderChanged();
}
} // namespace ui

@ -9,6 +9,7 @@
#include "base/observer_list.h"
#include "base/observer_list_types.h"
#include "ui/color/color_provider.h"
#include "ui/color/color_provider_manager.h"
namespace ui {
@ -29,6 +30,11 @@ class COMPONENT_EXPORT(COLOR) ColorProviderSource {
// this source.
virtual const ColorProvider* GetColorProvider() const = 0;
// Gets the ColorProviderKey associated with the source's current
// ColorProvider instance.
virtual ColorProviderManager::ColorProviderKey GetColorProviderKey()
const = 0;
void AddObserver(ColorProviderSourceObserver* observer);
void RemoveObserver(ColorProviderSourceObserver* observer);
@ -36,6 +42,10 @@ class COMPONENT_EXPORT(COLOR) ColorProviderSource {
// in the method `GetColorProvider()` changes.
void NotifyColorProviderChanged();
base::ObserverList<ColorProviderSourceObserver>& observers_for_testing() {
return observers_;
}
private:
base::ObserverList<ColorProviderSourceObserver> observers_;
};

@ -11,15 +11,30 @@ ColorProviderSourceObserver::ColorProviderSourceObserver() = default;
ColorProviderSourceObserver::~ColorProviderSourceObserver() = default;
void ColorProviderSourceObserver::OnColorProviderSourceDestroying() {
source_ = nullptr;
color_provider_source_observation_.Reset();
}
const ui::ColorProviderSource*
ColorProviderSourceObserver::GetColorProviderSourceForTesting() const {
return GetColorProviderSource();
}
void ColorProviderSourceObserver::Observe(ColorProviderSource* source) {
if (color_provider_source_observation_.IsObservingSource(source))
if (source && color_provider_source_observation_.IsObservingSource(source))
return;
color_provider_source_observation_.Reset();
source_ = source;
if (!source_)
return;
color_provider_source_observation_.Observe(source);
}
const ui::ColorProviderSource*
ColorProviderSourceObserver::GetColorProviderSource() const {
return source_;
}
} // namespace ui

@ -21,18 +21,27 @@ class COMPONENT_EXPORT(COLOR) ColorProviderSourceObserver
~ColorProviderSourceObserver() override;
// Called when the source's ColorProvider instance has changed.
virtual void OnColorProviderChanged(const ColorProviderSource* source) = 0;
// Starts observing the new `source`. Clears the current observation if
// already observing a ColorProviderSource.
void Observe(ColorProviderSource* source);
virtual void OnColorProviderChanged() = 0;
// Called by the ColorProviderSource during destruction. Avoids situations
// where we could be left with a dangling pointer should the observer outlive
// the source.
void OnColorProviderSourceDestroying();
const ui::ColorProviderSource* GetColorProviderSourceForTesting() const;
protected:
// Starts observing the new `source`. Clears the current observation if
// already observing a ColorProviderSource.
void Observe(ColorProviderSource* source);
// Gets the ColorProviderSource currently under observation, if it exists.
const ui::ColorProviderSource* GetColorProviderSource() const;
private:
// The currently observed source.
const ui::ColorProviderSource* source_ = nullptr;
// Ensure references to the observer are removed from the source should the
// source outlive the observer.
base::ScopedObservation<ColorProviderSource, ColorProviderSourceObserver>

@ -0,0 +1,127 @@
// Copyright 2021 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 "ui/color/color_provider_source.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/color/color_provider_manager.h"
#include "ui/color/color_provider_source_observer.h"
namespace ui {
namespace {
class MockColorProviderSource : public ColorProviderSource {
public:
MOCK_METHOD(const ColorProvider*, GetColorProvider, (), (const, override));
MOCK_METHOD(ColorProviderManager::ColorProviderKey,
GetColorProviderKey,
(),
(const, override));
};
class MockColorProviderSourceObserver : public ColorProviderSourceObserver {
public:
void ObserveForTesting(ColorProviderSource* source) { Observe(source); }
MOCK_METHOD(void, OnColorProviderChanged, (), ());
};
} // namespace
using ColorProviderSourceObserverTest = testing::Test;
// Verify the observation is reset when source is destroyed.
TEST_F(ColorProviderSourceObserverTest, DestroyingSourceClearsItFromObservers) {
auto source = std::make_unique<MockColorProviderSource>();
MockColorProviderSourceObserver observer_1;
MockColorProviderSourceObserver observer_2;
// OnColorProviderChanged() should be called once when the observer is first
// added to the source.
EXPECT_CALL(observer_1, OnColorProviderChanged()).Times(1);
EXPECT_CALL(observer_2, OnColorProviderChanged()).Times(1);
auto set_observaton = [&](MockColorProviderSourceObserver* observer) {
observer->ObserveForTesting(source.get());
EXPECT_EQ(source.get(), observer->GetColorProviderSourceForTesting());
EXPECT_TRUE(source->observers_for_testing().HasObserver(observer));
};
set_observaton(&observer_1);
set_observaton(&observer_2);
// When the source is destroyed the observer's source() method should return
// nullptr.
source.reset();
EXPECT_EQ(nullptr, observer_1.GetColorProviderSourceForTesting());
EXPECT_EQ(nullptr, observer_2.GetColorProviderSourceForTesting());
}
// Verify the observer is removed from the source's observer list when the
// observer is destroyed.
TEST_F(ColorProviderSourceObserverTest, DestroyingObserverClearsItFromSource) {
MockColorProviderSource source;
auto observer_1 = std::make_unique<MockColorProviderSourceObserver>();
auto observer_2 = std::make_unique<MockColorProviderSourceObserver>();
// OnColorProviderChanged() should be called once when the observer is first
// added to the source.
EXPECT_CALL(*observer_1, OnColorProviderChanged()).Times(1);
EXPECT_CALL(*observer_2, OnColorProviderChanged()).Times(1);
auto set_observaton = [&](MockColorProviderSourceObserver* observer) {
observer->ObserveForTesting(&source);
EXPECT_EQ(&source, observer->GetColorProviderSourceForTesting());
EXPECT_TRUE(source.observers_for_testing().HasObserver(observer));
};
set_observaton(observer_1.get());
set_observaton(observer_2.get());
// When the observer is destroyed it should be removed from the source's list
// of observers. Other observers should remain.
observer_1.reset();
EXPECT_FALSE(source.observers_for_testing().empty());
EXPECT_TRUE(source.observers_for_testing().HasObserver(observer_2.get()));
observer_2.reset();
EXPECT_TRUE(source.observers_for_testing().empty());
// Further calls to NotifyColorProviderChanged() should succeed and not
// result in any more calls to OnColorProviderChanged().
source.NotifyColorProviderChanged();
}
// Verify OnColorProviderChanged() is called by the source as expected.
TEST_F(ColorProviderSourceObserverTest,
ObserverCorrectlySetsObservationOfSource) {
MockColorProviderSource source;
MockColorProviderSourceObserver observer_1;
MockColorProviderSourceObserver observer_2;
// observer_2 should not be notified after resetting its observation below.
EXPECT_CALL(observer_1, OnColorProviderChanged()).Times(4);
EXPECT_CALL(observer_2, OnColorProviderChanged()).Times(2);
auto set_observaton_and_notify =
[&](MockColorProviderSourceObserver* observer) {
observer->ObserveForTesting(&source);
EXPECT_EQ(&source, observer->GetColorProviderSourceForTesting());
EXPECT_TRUE(source.observers_for_testing().HasObserver(observer));
source.NotifyColorProviderChanged();
};
set_observaton_and_notify(&observer_1);
set_observaton_and_notify(&observer_2);
observer_2.ObserveForTesting(nullptr);
// After removing the observation for observer_2 we should still get
// notifications for observer_1.
EXPECT_EQ(&source, observer_1.GetColorProviderSourceForTesting());
EXPECT_EQ(nullptr, observer_2.GetColorProviderSourceForTesting());
EXPECT_TRUE(source.observers_for_testing().HasObserver(&observer_1));
EXPECT_FALSE(source.observers_for_testing().HasObserver(&observer_2));
source.NotifyColorProviderChanged();
}
} // namespace ui

@ -88,9 +88,14 @@ void WebView::SetWebContents(content::WebContents* replacement) {
TRACE_EVENT0("views", "WebView::SetWebContents");
if (replacement == web_contents())
return;
if (web_contents())
web_contents()->SetColorProviderSource(nullptr);
SetCrashedOverlayView(nullptr);
DetachWebContentsNativeView();
WebContentsObserver::Observe(replacement);
if (replacement)
replacement->SetColorProviderSource(GetWidget());
// web_contents() now returns |replacement| from here onwards.
if (wc_owner_.get() != replacement)
wc_owner_.reset();
@ -256,10 +261,15 @@ void WebView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
}
void WebView::AddedToWidget() {
if (!web_contents())
return;
web_contents()->SetColorProviderSource(GetWidget());
// If added to a widget hierarchy and |holder_| already has a NativeView
// attached, update the accessible parent here to support reparenting the
// WebView.
if (web_contents() && holder_->native_view())
if (holder_->native_view())
UpdateNativeViewHostAccessibleParent(holder_, parent());
}

@ -1742,17 +1742,22 @@ void Widget::OnNativeThemeUpdated(ui::NativeTheme* observed_theme) {
// Widget, ui::ColorProviderSource:
const ui::ColorProvider* Widget::GetColorProvider() const {
auto color_scheme = GetNativeTheme()->GetDefaultSystemColorScheme();
return ui::ColorProviderManager::Get().GetColorProviderFor(
{(color_scheme == ui::NativeTheme::ColorScheme::kDark)
? ui::ColorProviderManager::ColorMode::kDark
: ui::ColorProviderManager::ColorMode::kLight,
(color_scheme == ui::NativeTheme::ColorScheme::kPlatformHighContrast)
? ui::ColorProviderManager::ContrastMode::kHigh
: ui::ColorProviderManager::ContrastMode::kNormal,
GetNativeTheme()->is_custom_system_theme()
? ui::ColorProviderManager::SystemTheme::kCustom
: ui::ColorProviderManager::SystemTheme::kDefault});
GetColorProviderKey());
}
ui::ColorProviderManager::ColorProviderKey Widget::GetColorProviderKey() const {
const auto* native_theme = GetNativeTheme();
const auto color_scheme = native_theme->GetDefaultSystemColorScheme();
return {(color_scheme == ui::NativeTheme::ColorScheme::kDark)
? ui::ColorProviderManager::ColorMode::kDark
: ui::ColorProviderManager::ColorMode::kLight,
(color_scheme == ui::NativeTheme::ColorScheme::kPlatformHighContrast)
? ui::ColorProviderManager::ContrastMode::kHigh
: ui::ColorProviderManager::ContrastMode::kNormal,
native_theme->is_custom_system_theme()
? ui::ColorProviderManager::SystemTheme::kCustom
: ui::ColorProviderManager::SystemTheme::kDefault};
}
////////////////////////////////////////////////////////////////////////////////

@ -1040,6 +1040,8 @@ class VIEWS_EXPORT Widget : public internal::NativeWidgetDelegate,
// ui::ColorProviderSource:
const ui::ColorProvider* GetColorProvider() const override;
ui::ColorProviderManager::ColorProviderKey GetColorProviderKey()
const override;
// Set the native theme from which this widget gets color from.
void SetNativeThemeForTest(ui::NativeTheme* native_theme) {