Move DidChangeThemeColor mojo definition to blink.

This change moves the definition of DidChangeThemeColor from FrameHost to LocalFrameHost in blink. It removes the public definition on the WebLocalFrameClient as it isn't necessary anymore. BUG=1008432 Change-Id: I7c78c34d0dc4b81733b649dc8b3bf8aab334f3e9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1899321 Commit-Queue: Dave Tapuska <[email protected]> Reviewed-by: Avi Drissman <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Cr-Commit-Position: refs/heads/master@{#713929}
xuefu · Nov 8, 2019 · 9359442 · 9359442
1 parent 7e7c1ff
commit 9359442
Show file tree

Hide file tree

Showing 18 changed files with 64 additions and 54 deletions.
diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h
@@ -995,10 +995,6 @@ class CONTENT_EXPORT RenderFrameHostImpl
                               const url::Origin& source_origin,
                               const base::Optional<url::Origin>& target_origin);
 
-  // mojom::FrameHost:
-  void VisibilityChanged(blink::mojom::FrameVisibility) override;
-  void DidChangeThemeColor(const base::Optional<SkColor>& theme_color) override;
-
   blink::mojom::FrameVisibility visibility() const { return visibility_; }
 
   // A CommitCallbackInterceptor is used to modify parameters for or cancel a
@@ -1206,6 +1202,10 @@ class CONTENT_EXPORT RenderFrameHostImpl
   void DidDisplayInsecureContent() override;
   void DidContainInsecureFormAction() override;
   void SetNeedsOcclusionTracking(bool needs_tracking) override;
+  void LifecycleStateChanged(blink::mojom::FrameLifecycleState state) override;
+  void EvictFromBackForwardCache() override;
+  void VisibilityChanged(blink::mojom::FrameVisibility) override;
+  void DidChangeThemeColor(const base::Optional<SkColor>& theme_color) override;
 
  protected:
   friend class RenderFrameHostFactory;
@@ -1457,7 +1457,6 @@ class CONTENT_EXPORT RenderFrameHostImpl
           validated_params,
       mojom::DidCommitProvisionalLoadInterfaceParamsPtr interface_params)
       override;
-  void EvictFromBackForwardCache() override;
 
   // This function mimics DidCommitProvisionalLoad but is a direct mojo
   // callback from NavigationClient::CommitNavigation.
@@ -1494,7 +1493,6 @@ class CONTENT_EXPORT RenderFrameHostImpl
   void CancelInitialHistoryLoad() override;
   void UpdateEncoding(const std::string& encoding) override;
   void FrameSizeChanged(const gfx::Size& frame_size) override;
-  void LifecycleStateChanged(blink::mojom::FrameLifecycleState state) override;
   void DocumentOnLoadCompleted() override;
   void UpdateActiveSchedulerTrackedFeatures(uint64_t features_mask) override;
   void DidAddMessageToConsole(blink::mojom::ConsoleMessageLevel log_level,

diff --git a/content/common/frame.mojom b/content/common/frame.mojom
@@ -22,7 +22,6 @@ 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";
@@ -513,7 +512,4 @@ interface FrameHost {
 
   // Sent by the renderer when the frame becomes focused.
   FrameFocused();
-
-  // Notifies the browser that the current frame has changed theme color.
-  DidChangeThemeColor(skia.mojom.SkColor? theme_color);
 };
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
@@ -4837,13 +4837,6 @@ void RenderFrameImpl::DidUpdateCurrentHistoryItem() {
   render_view_->StartNavStateSyncTimerIfNecessary(this);
 }
 
-void RenderFrameImpl::DidChangeThemeColor() {
-  if (frame_->Parent())
-    return;
-
-  GetFrameHost()->DidChangeThemeColor(frame_->GetDocument().ThemeColor());
-}
-
 void RenderFrameImpl::ForwardResourceTimingToParent(
     const blink::WebResourceTimingInfo& info) {
   Send(new FrameHostMsg_ForwardResourceTimingToParent(

diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h
@@ -739,7 +739,6 @@ class CONTENT_EXPORT RenderFrameImpl
                                        blink::WebHistoryCommitType commit_type,
                                        bool content_initiated) override;
   void DidUpdateCurrentHistoryItem() override;
-  void DidChangeThemeColor() override;
   void ForwardResourceTimingToParent(
       const blink::WebResourceTimingInfo& info) override;
   void DispatchLoad() override;

diff --git a/content/test/test_render_frame.cc b/content/test/test_render_frame.cc
@@ -194,9 +194,6 @@ class MockFrameHost : public mojom::FrameHost {
   void UpdateUserGestureCarryoverInfo() override {}
 #endif
 
-  void DidChangeThemeColor(
-      const base::Optional<::SkColor>& theme_color) override {}
-
  private:
   std::unique_ptr<FrameHostMsg_DidCommitProvisionalLoad_Params>
       last_commit_params_;

diff --git a/third_party/blink/public/mojom/frame/frame.mojom b/third_party/blink/public/mojom/frame/frame.mojom
@@ -6,6 +6,7 @@ module blink.mojom;
 
 import "cc/mojom/touch_action.mojom";
 import "mojo/public/mojom/base/string16.mojom";
+import "skia/public/mojom/skcolor.mojom";
 import "third_party/blink/public/mojom/devtools/console_message.mojom";
 import "third_party/blink/public/mojom/frame/fullscreen.mojom";
 import "third_party/blink/public/mojom/frame/lifecycle.mojom";
@@ -74,6 +75,12 @@ interface LocalFrameHost {
   // of the viewport, or the need for a layout object changes, e.g. if the
   // frame owner assigns a display: none style.
   VisibilityChanged(blink.mojom.FrameVisibility visibility);
+
+  // Notifies the browser that the associated frame has changed theme color.
+  // This will only be called on main-frames only. |theme_color| is optional
+  // and indicates if a theme color has been specified or not, e.g. removal
+  // of a theme color meta tag  will generate a null value.
+  DidChangeThemeColor(skia.mojom.SkColor? theme_color);
 };
 
 // Implemented in Blink, this interface defines frame-specific methods that will

diff --git a/third_party/blink/public/web/web_local_frame_client.h b/third_party/blink/public/web/web_local_frame_client.h
@@ -471,9 +471,6 @@ class BLINK_EXPORT WebLocalFrameClient {
   // WARNING: This method may be called very frequently.
   virtual void DidUpdateCurrentHistoryItem() {}
 
-  // The frame's theme color has changed.
-  virtual void DidChangeThemeColor() {}
-
   // Called to report resource timing information for this frame to the parent.
   // Only used when the parent frame is remote.
   virtual void ForwardResourceTimingToParent(const WebResourceTimingInfo&) {}

diff --git a/third_party/blink/renderer/core/DEPS b/third_party/blink/renderer/core/DEPS
@@ -66,7 +66,7 @@ include_rules = [
     "+services/network/public/cpp/shared_url_loader_factory.h",
     "+services/resource_coordinator/public/mojom/coordination_unit.mojom-blink.h",
     "+services/service_manager/public",
-    "+skia/public/mojom/bitmap_skbitmap_mojom_traits.h",
+    "+skia/public/mojom",
     "+skia/ext/image_operations.h",
     "+skia/ext/skia_utils_mac.h",
     "+third_party/blink/public/common",

diff --git a/third_party/blink/renderer/core/exported/local_frame_client_impl.cc b/third_party/blink/renderer/core/exported/local_frame_client_impl.cc
@@ -462,11 +462,6 @@ void LocalFrameClientImpl::DispatchDidFinishLoad() {
   web_frame_->DidFinish();
 }
 
-void LocalFrameClientImpl::DispatchDidChangeThemeColor() {
-  if (web_frame_->Client())
-    web_frame_->Client()->DidChangeThemeColor();
-}
-
 void LocalFrameClientImpl::BeginNavigation(
     const ResourceRequest& request,
     network::mojom::RequestContextFrameType frame_type,

diff --git a/third_party/blink/renderer/core/exported/local_frame_client_impl.h b/third_party/blink/renderer/core/exported/local_frame_client_impl.h
@@ -112,7 +112,6 @@ class LocalFrameClientImpl final : public LocalFrameClient {
   void DispatchDidFinishDocumentLoad() override;
   void DispatchDidFinishLoad() override;
 
-  void DispatchDidChangeThemeColor() override;
   void BeginNavigation(
       const ResourceRequest&,
       network::mojom::RequestContextFrameType,

diff --git a/third_party/blink/renderer/core/exported/web_frame_test.cc b/third_party/blink/renderer/core/exported/web_frame_test.cc
@@ -42,6 +42,7 @@
 #include "mojo/public/cpp/bindings/binding.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver.h"
+#include "skia/public/mojom/skcolor.mojom-blink.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/common/context_menu_data/edit_flags.h"
@@ -8750,64 +8751,73 @@ TEST_F(WebFrameTest, PrintingBasic)
   frame->PrintEnd();
 }
 
-class ThemeColorTestWebFrameClient
-    : public frame_test_helpers::TestWebFrameClient {
+class ThemeColorTestLocalFrameHost : public FakeLocalFrameHost {
  public:
-  ThemeColorTestWebFrameClient() : did_notify_(false) {}
-  ~ThemeColorTestWebFrameClient() override = default;
+  ThemeColorTestLocalFrameHost() = default;
+  ~ThemeColorTestLocalFrameHost() override = default;
 
   void Reset() { did_notify_ = false; }
 
   bool DidNotify() const { return did_notify_; }
 
  private:
-  // frame_test_helpers::TestWebFrameClient:
-  void DidChangeThemeColor() override { did_notify_ = true; }
+  // FakeLocalFrameHost:
+  void DidChangeThemeColor(
+      const base::Optional<::SkColor>& theme_color) override {
+    did_notify_ = true;
+  }
 
-  bool did_notify_;
+  bool did_notify_ = false;
 };
 
 TEST_F(WebFrameTest, ThemeColor) {
   RegisterMockedHttpURLLoad("theme_color_test.html");
-  ThemeColorTestWebFrameClient client;
+  ThemeColorTestLocalFrameHost host;
+  frame_test_helpers::TestWebFrameClient client;
+  host.Init(client.GetRemoteNavigationAssociatedInterfaces());
   frame_test_helpers::WebViewHelper web_view_helper;
   web_view_helper.InitializeAndLoad(base_url_ + "theme_color_test.html",
                                     &client);
-  EXPECT_TRUE(client.DidNotify());
+  EXPECT_TRUE(host.DidNotify());
   WebLocalFrameImpl* frame = web_view_helper.LocalMainFrame();
   EXPECT_EQ(Color(0, 0, 255), frame->GetDocument().ThemeColor());
   // Change color by rgb.
-  client.Reset();
+  host.Reset();
   frame->ExecuteScript(
       WebScriptSource("document.getElementById('tc1').setAttribute('content', "
                       "'rgb(0, 0, 0)');"));
-  EXPECT_TRUE(client.DidNotify());
+  RunPendingTasks();
+  EXPECT_TRUE(host.DidNotify());
   EXPECT_EQ(Color::kBlack, frame->GetDocument().ThemeColor());
   // Change color by hsl.
-  client.Reset();
+  host.Reset();
   frame->ExecuteScript(
       WebScriptSource("document.getElementById('tc1').setAttribute('content', "
                       "'hsl(240,100%, 50%)');"));
-  EXPECT_TRUE(client.DidNotify());
+  RunPendingTasks();
+  EXPECT_TRUE(host.DidNotify());
   EXPECT_EQ(Color(0, 0, 255), frame->GetDocument().ThemeColor());
   // Change of second theme-color meta tag will not change frame's theme
   // color.
-  client.Reset();
+  host.Reset();
   frame->ExecuteScript(WebScriptSource(
       "document.getElementById('tc2').setAttribute('content', '#00FF00');"));
-  EXPECT_TRUE(client.DidNotify());
+  RunPendingTasks();
+  EXPECT_TRUE(host.DidNotify());
   EXPECT_EQ(Color(0, 0, 255), frame->GetDocument().ThemeColor());
   // Remove the first theme-color meta tag to apply the second.
-  client.Reset();
+  host.Reset();
   frame->ExecuteScript(
       WebScriptSource("document.getElementById('tc1').remove();"));
-  EXPECT_TRUE(client.DidNotify());
+  RunPendingTasks();
+  EXPECT_TRUE(host.DidNotify());
   EXPECT_EQ(Color(0, 255, 0), frame->GetDocument().ThemeColor());
   // Remove the name attribute of the remaining meta.
-  client.Reset();
+  host.Reset();
   frame->ExecuteScript(WebScriptSource(
       "document.getElementById('tc2').removeAttribute('name');"));
-  EXPECT_TRUE(client.DidNotify());
+  RunPendingTasks();
+  EXPECT_TRUE(host.DidNotify());
   EXPECT_EQ(base::nullopt, frame->GetDocument().ThemeColor());
 }
 

diff --git a/third_party/blink/renderer/core/frame/local_frame.cc b/third_party/blink/renderer/core/frame/local_frame.cc
@@ -36,6 +36,7 @@
 
 #include "services/network/public/cpp/features.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
+#include "skia/public/mojom/skcolor.mojom-blink.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 #include "third_party/blink/public/common/browser_interface_broker_proxy.h"
 #include "third_party/blink/public/common/frame/blocked_navigation_types.h"
@@ -562,6 +563,18 @@ bool LocalFrame::BubbleLogicalScrollFromChildFrame(
                                           owner_element);
 }
 
+void LocalFrame::DidChangeThemeColor() {
+  if (Tree().Parent())
+    return;
+
+  base::Optional<Color> color = GetDocument()->ThemeColor();
+  base::Optional<SkColor> sk_color;
+  if (color)
+    sk_color = color->Rgb();
+
+  GetLocalFrameHostRemote().DidChangeThemeColor(sk_color);
+}
+
 LocalFrame& LocalFrame::LocalFrameRoot() const {
   const LocalFrame* cur_frame = this;
   while (cur_frame && IsA<LocalFrame>(cur_frame->Tree().Parent()))

diff --git a/third_party/blink/renderer/core/frame/local_frame.h b/third_party/blink/renderer/core/frame/local_frame.h
@@ -154,6 +154,8 @@ class CORE_EXPORT LocalFrame final : public Frame,
                                          ScrollGranularity granularity,
                                          Frame* child) override;
 
+  void DidChangeThemeColor();
+
   void DetachChildren();
   // After Document is attached, resets state related to document, and sets
   // context to the current document.

diff --git a/third_party/blink/renderer/core/frame/local_frame_client.h b/third_party/blink/renderer/core/frame/local_frame_client.h
@@ -146,7 +146,6 @@ class CORE_EXPORT LocalFrameClient : public FrameClient {
                                    WebHistoryCommitType) = 0;
   virtual void DispatchDidFinishDocumentLoad() = 0;
   virtual void DispatchDidFinishLoad() = 0;
-  virtual void DispatchDidChangeThemeColor() = 0;
 
   virtual void BeginNavigation(
       const ResourceRequest&,

diff --git a/third_party/blink/renderer/core/html/html_meta_element.cc b/third_party/blink/renderer/core/html/html_meta_element.cc
@@ -480,7 +480,7 @@ void HTMLMetaElement::NameRemoved(const AtomicString& name_value) {
     return;
   if (EqualIgnoringASCIICase(name_value, "theme-color") &&
       GetDocument().GetFrame()) {
-    GetDocument().GetFrame()->Client()->DispatchDidChangeThemeColor();
+    GetDocument().GetFrame()->DidChangeThemeColor();
   } else if (EqualIgnoringASCIICase(name_value, "color-scheme")) {
     GetDocument().ColorSchemeMetaChanged();
   }
@@ -557,7 +557,7 @@ void HTMLMetaElement::ProcessContent() {
 
   if (EqualIgnoringASCIICase(name_value, "theme-color") &&
       GetDocument().GetFrame()) {
-    GetDocument().GetFrame()->Client()->DispatchDidChangeThemeColor();
+    GetDocument().GetFrame()->DidChangeThemeColor();
     return;
   }
   if (EqualIgnoringASCIICase(name_value, "color-scheme")) {

diff --git a/third_party/blink/renderer/core/loader/empty_clients.h b/third_party/blink/renderer/core/loader/empty_clients.h
@@ -285,7 +285,6 @@ class CORE_EXPORT EmptyLocalFrameClient : public LocalFrameClient {
                            WebHistoryCommitType) override {}
   void DispatchDidFinishDocumentLoad() override {}
   void DispatchDidFinishLoad() override {}
-  void DispatchDidChangeThemeColor() override {}
 
   void BeginNavigation(
       const ResourceRequest&,

diff --git a/third_party/blink/renderer/core/testing/fake_local_frame_host.cc b/third_party/blink/renderer/core/testing/fake_local_frame_host.cc
@@ -4,6 +4,7 @@
 
 #include "third_party/blink/renderer/core/testing/fake_local_frame_host.h"
 
+#include "skia/public/mojom/skcolor.mojom-blink.h"
 #include "third_party/blink/public/mojom/frame/fullscreen.mojom-blink.h"
 
 namespace blink {
@@ -45,6 +46,9 @@ void FakeLocalFrameHost::EvictFromBackForwardCache() {}
 void FakeLocalFrameHost::VisibilityChanged(
     mojom::blink::FrameVisibility visibility) {}
 
+void FakeLocalFrameHost::DidChangeThemeColor(
+    const base::Optional<::SkColor>& theme_color) {}
+
 void FakeLocalFrameHost::BindFrameHostReceiver(
     mojo::ScopedInterfaceEndpointHandle handle) {
   receiver_.Bind(mojo::PendingAssociatedReceiver<mojom::blink::LocalFrameHost>(

diff --git a/third_party/blink/renderer/core/testing/fake_local_frame_host.h b/third_party/blink/renderer/core/testing/fake_local_frame_host.h
@@ -37,6 +37,8 @@ class FakeLocalFrameHost : public mojom::blink::LocalFrameHost {
   void LifecycleStateChanged(mojom::blink::FrameLifecycleState state) override;
   void EvictFromBackForwardCache() override;
   void VisibilityChanged(mojom::blink::FrameVisibility visibility) override;
+  void DidChangeThemeColor(
+      const base::Optional<::SkColor>& theme_color) override;
 
  private:
   void BindFrameHostReceiver(mojo::ScopedInterfaceEndpointHandle handle);