Skip to content
Permalink
Browse files

Migrate legacy FrameHostMsg_DidChangeThemeColor IPC

Also send the event to the RenderFrameHostImpl. In the future we can use
this to store the theme_color in the RenderFrameHostImpl instead of doing
it in the WebContentsImpl, which will make it easier to persist the
color when the page goes into the BackForwardCache

TBR=mtklein@google.com

Bug: 1001087
Change-Id: Ifdb81f8b7e71f41801b7bb32a8d1544eed9ed302
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1821903
Commit-Queue: Alexander Timin <altimin@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705684}
  • Loading branch information
Alexander Timin Commit Bot
Alexander Timin authored and Commit Bot committed Oct 14, 2019
1 parent 747a6c9 commit a40ce5a36aa294a1859a2004412fa079c0a36953
@@ -187,6 +187,7 @@ jumbo_source_set("browser") {
"//services/viz/public/cpp/gpu",
"//services/viz/public/mojom",
"//skia",
"//skia/public/mojom",
"//sql",
"//storage/browser",
"//storage/common",
@@ -11,6 +11,7 @@
#include <vector>

#include "base/i18n/rtl.h"
#include "base/optional.h"
#include "build/build_config.h"
#include "components/viz/common/surfaces/surface_id.h"
#include "content/browser/webui/web_ui_impl.h"
@@ -31,6 +32,7 @@
#include "third_party/blink/public/common/frame/blocked_navigation_types.h"
#include "third_party/blink/public/common/mediastream/media_stream_request.h"
#include "third_party/blink/public/mojom/devtools/console_message.mojom.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/accessibility/ax_mode.h"
#include "ui/base/window_open_disposition.h"

@@ -449,6 +451,11 @@ class CONTENT_EXPORT RenderFrameHostDelegate {
virtual RenderFrameHostImpl* GetMainFrameForInnerDelegate(
FrameTreeNode* frame_tree_node);

// Notifies that the given frame has changed theme color.
virtual void OnThemeColorChanged(RenderFrameHostImpl* source,
const base::Optional<SkColor>& theme_color) {
}

protected:
virtual ~RenderFrameHostDelegate() {}
};
@@ -3375,6 +3375,11 @@ void RenderFrameHostImpl::VisibilityChanged(
UpdateFrameFrozenState();
}

void RenderFrameHostImpl::DidChangeThemeColor(
const base::Optional<SkColor>& theme_color) {
delegate_->OnThemeColorChanged(this, theme_color);
}

void RenderFrameHostImpl::SetCommitCallbackInterceptorForTesting(
CommitCallbackInterceptor* interceptor) {
// This DCHECK's aims to avoid unexpected replacement of an interceptor.
@@ -106,6 +106,7 @@
#include "third_party/blink/public/platform/web_sudden_termination_disabler_type.h"
#include "third_party/blink/public/web/web_text_direction.h"
#include "third_party/blink/public/web/web_tree_scope_type.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/accessibility/ax_action_handler.h"
#include "ui/accessibility/ax_mode.h"
#include "ui/accessibility/ax_node_data.h"
@@ -1011,6 +1012,7 @@ class CONTENT_EXPORT RenderFrameHostImpl

// mojom::FrameHost:
void VisibilityChanged(blink::mojom::FrameVisibility) override;
void DidChangeThemeColor(const base::Optional<SkColor>& theme_color) override;

blink::mojom::FrameVisibility visibility() const { return visibility_; }

@@ -903,8 +903,6 @@ bool WebContentsImpl::OnMessageReceived(RenderFrameHostImpl* render_frame_host,
IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(WebContentsImpl, message, render_frame_host)
IPC_MESSAGE_HANDLER(FrameHostMsg_DomOperationResponse,
OnDomOperationResponse)
IPC_MESSAGE_HANDLER(FrameHostMsg_DidChangeThemeColor,
OnThemeColorChanged)
IPC_MESSAGE_HANDLER(FrameHostMsg_DidFinishLoad, OnDidFinishLoad)
IPC_MESSAGE_HANDLER(FrameHostMsg_DidLoadResourceFromMemoryCache,
OnDidLoadResourceFromMemoryCache)
@@ -4629,8 +4627,9 @@ bool WebContentsImpl::CanOverscrollContent() const {
return false;
}

void WebContentsImpl::OnThemeColorChanged(RenderFrameHostImpl* source,
base::Optional<SkColor> theme_color) {
void WebContentsImpl::OnThemeColorChanged(
RenderFrameHostImpl* source,
const base::Optional<SkColor>& theme_color) {
if (source != GetMainFrame()) {
// Only the main frame may control the theme.
return;
@@ -636,6 +636,8 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
int context_id) override;
RenderFrameHostImpl* GetMainFrameForInnerDelegate(
FrameTreeNode* frame_tree_node) override;
void OnThemeColorChanged(RenderFrameHostImpl* source,
const base::Optional<SkColor>& theme_color) override;

// RenderViewHostDelegate ----------------------------------------------------
RenderViewHostDelegateView* GetDelegateView() override;
@@ -1282,8 +1284,6 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
const base::string16& user_input);

// IPC message handlers.
void OnThemeColorChanged(RenderFrameHostImpl* source,
base::Optional<SkColor> theme_color);
void OnDidLoadResourceFromMemoryCache(
RenderFrameHostImpl* source,
const GURL& url,
@@ -3260,8 +3260,7 @@ TEST_F(WebContentsImplTest, ThemeColorChangeDependingOnFirstVisiblePaint) {

// Theme color changes should not propagate past the WebContentsImpl before
// the first visually non-empty paint has occurred.
rfh->OnMessageReceived(
FrameHostMsg_DidChangeThemeColor(rfh->GetRoutingID(), SK_ColorRED));
rfh->DidChangeThemeColor(SK_ColorRED);

EXPECT_EQ(SK_ColorRED, contents()->GetThemeColor());
EXPECT_EQ(base::nullopt, observer.last_theme_color());
@@ -3274,8 +3273,7 @@ TEST_F(WebContentsImplTest, ThemeColorChangeDependingOnFirstVisiblePaint) {
EXPECT_EQ(SK_ColorRED, observer.last_theme_color());

// Additional changes made by the web contents should propagate as well.
rfh->OnMessageReceived(
FrameHostMsg_DidChangeThemeColor(rfh->GetRoutingID(), SK_ColorGREEN));
rfh->DidChangeThemeColor(SK_ColorGREEN);

EXPECT_EQ(SK_ColorGREEN, contents()->GetThemeColor());
EXPECT_EQ(SK_ColorGREEN, observer.last_theme_color());
@@ -22,6 +22,7 @@ import "services/network/public/mojom/url_loader.mojom";
import "services/network/public/mojom/url_loader_factory.mojom";
import "services/service_manager/public/mojom/interface_provider.mojom";
import "services/viz/public/mojom/compositing/surface_id.mojom";
import "skia/public/mojom/skcolor.mojom";
import "third_party/blink/public/mojom/blob/blob_url_store.mojom";
import "third_party/blink/public/mojom/commit_result/commit_result.mojom";
import "third_party/blink/public/mojom/devtools/console_message.mojom";
@@ -566,4 +567,7 @@ interface FrameHost {
// Evicts the page from the back/forward cache due to e.g., JavaScript
// execution.
EvictFromBackForwardCache();

// Notifies the browser that the current frame has changed theme color.
DidChangeThemeColor(skia.mojom.SkColor? theme_color);
};
@@ -1328,10 +1328,6 @@ IPC_SYNC_MESSAGE_ROUTED1_2(FrameHostMsg_RunBeforeUnloadConfirm,
bool /* out - success */,
base::string16 /* out - This is ignored.*/)

// Notify browser the theme color has been changed.
IPC_MESSAGE_ROUTED1(FrameHostMsg_DidChangeThemeColor,
base::Optional<SkColor> /* theme_color */)

// Register a new handler for URL requests with the given scheme.
IPC_MESSAGE_ROUTED4(FrameHostMsg_RegisterProtocolHandler,
std::string /* scheme */,
@@ -5158,8 +5158,7 @@ void RenderFrameImpl::DidChangeThemeColor() {
if (frame_->Parent())
return;

Send(new FrameHostMsg_DidChangeThemeColor(
routing_id_, frame_->GetDocument().ThemeColor()));
GetFrameHost()->DidChangeThemeColor(frame_->GetDocument().ThemeColor());
}

void RenderFrameImpl::ForwardResourceTimingToParent(
@@ -9,6 +9,7 @@
#include <vector>

#include "base/bind_helpers.h"
#include "base/optional.h"
#include "base/threading/thread_task_runner_handle.h"
#include "build/build_config.h"
#include "content/common/frame.mojom.h"
@@ -29,6 +30,7 @@
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
#include "third_party/blink/public/web/web_local_frame.h"
#include "third_party/blink/public/web/web_navigation_control.h"
#include "third_party/skia/include/core/SkColor.h"

namespace content {

@@ -227,6 +229,9 @@ class MockFrameHost : public mojom::FrameHost {

void EvictFromBackForwardCache() override {}

void DidChangeThemeColor(
const base::Optional<::SkColor>& theme_color) override {}

private:
std::unique_ptr<FrameHostMsg_DidCommitProvisionalLoad_Params>
last_commit_params_;
@@ -9,6 +9,7 @@ mojom("mojom") {
"bitmap.mojom",
"blur_image_filter_tile_mode.mojom",
"image_info.mojom",
"skcolor.mojom",
]

public_deps = [
@@ -0,0 +1,10 @@
// 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.

module skia.mojom;

// Mirror of SkColor
struct SkColor {
uint32 value;
};
@@ -0,0 +1,11 @@
# 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.

mojom = "//skia/public/mojom/skcolor.mojom"
public_headers = [ "//third_party/skia/include/core/SkColor.h" ]
traits_headers = [ "//skia/public/mojom/skcolor_mojom_traits.h" ]
sources = [
"//skia/public/mojom/skcolor_mojom_traits.h",
]
type_mappings = [ "skia.mojom.SkColor=::SkColor" ]
@@ -0,0 +1,24 @@
// 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.

#ifndef SKIA_PUBLIC_MOJOM_SKCOLOR_MOJOM_TRAITS_H_
#define SKIA_PUBLIC_MOJOM_SKCOLOR_MOJOM_TRAITS_H_

#include "skia/public/mojom/skcolor.mojom.h"
#include "third_party/skia/include/core/SkColor.h"

namespace mojo {

template <>
struct StructTraits<skia::mojom::SkColorDataView, ::SkColor> {
static uint32_t value(::SkColor color) { return color; }
static bool Read(skia::mojom::SkColorDataView data, ::SkColor* color) {
*color = data.value();
return true;
}
};

} // namespace mojo

#endif // SKIA_PUBLIC_MOJOM_SKCOLOR_MOJOM_TRAITS_H_
@@ -5,5 +5,6 @@
typemaps = [
"//skia/public/mojom/blur_image_filter_tile_mode.typemap",
"//skia/public/mojom/skbitmap.typemap",
"//skia/public/mojom/skcolor.typemap",
"//skia/public/mojom/skimageinfo.typemap",
]

0 comments on commit a40ce5a

Please sign in to comment.
You can’t perform that action at this time.