Propagate theme color updates to the WebContentsDelegates.

When the theme color on a page changes after the first visually non-
empty paint, this should be propagated to the delegates so that the
browser UI can be updated accordingly.

BUG=474371

Review URL: https://codereview.chromium.org/1149073006

Cr-Commit-Position: refs/heads/master@{#332180}

3 files changed, 62 insertions, 3 deletions

diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index b9d8592..f92180f 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -315,6 +315,7 @@ WebContentsImpl::WebContentsImpl(BrowserContext* browser_context,
       has_accessed_initial_document_(false),
       theme_color_(SK_ColorTRANSPARENT),
       last_sent_theme_color_(SK_ColorTRANSPARENT),
+      did_first_visually_non_empty_paint_(false),
       capturer_count_(0),
       should_normally_be_visible_(true),
       is_being_destroyed_(false),
@@ -2735,6 +2736,8 @@ void WebContentsImpl::DidNavigateMainFramePostCommit(
     if (rwhvb)
       rwhvb->OnDidNavigateMainFrameToNewPage();
 
+    did_first_visually_non_empty_paint_ = false;
+
     // Reset theme color on navigation to new page.
     theme_color_ = SK_ColorTRANSPARENT;
   }
@@ -2789,9 +2792,16 @@ bool WebContentsImpl::CanOverscrollContent() const {
 }
 
 void WebContentsImpl::OnThemeColorChanged(SkColor theme_color) {
-  // Update the theme color. This is to be published to observers on visually
-  // non empty paint.
+  // Update the theme color. This is to be published to observers after the
+  // first visually non-empty paint.
   theme_color_ = theme_color;
+
+  if (did_first_visually_non_empty_paint_ &&
+      last_sent_theme_color_ != theme_color_) {
+    FOR_EACH_OBSERVER(WebContentsObserver, observers_,
+                      DidChangeThemeColor(theme_color_));
+    last_sent_theme_color_ = theme_color_;
+  }
 }
 
 void WebContentsImpl::OnDidLoadResourceFromMemoryCache(
@@ -3190,6 +3200,8 @@ void WebContentsImpl::OnFirstVisuallyNonEmptyPaint() {
   FOR_EACH_OBSERVER(WebContentsObserver, observers_,
                     DidFirstVisuallyNonEmptyPaint());
 
+  did_first_visually_non_empty_paint_ = true;
+
   if (theme_color_ != last_sent_theme_color_) {
     // Theme color should have updated by now if there was one.
     FOR_EACH_OBSERVER(WebContentsObserver, observers_,
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index 51efdb4..a6c6211 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -1112,6 +1112,9 @@ class CONTENT_EXPORT WebContentsImpl
   // The last published theme color.
   SkColor last_sent_theme_color_;
 
+  // Whether the first visually non-empty paint has occurred.
+  bool did_first_visually_non_empty_paint_;
+
   // Data for misc internal state ----------------------------------------------
 
   // When > 0, the WebContents is currently being captured (e.g., for
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index 7829b5a..b4747a5 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -39,6 +39,7 @@
 #include "content/test/test_render_view_host.h"
 #include "content/test/test_web_contents.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/core/SkColor.h"
 
 namespace content {
 namespace {
@@ -244,7 +245,8 @@ class WebContentsImplTest : public RenderViewHostImplTestHarness {
 class TestWebContentsObserver : public WebContentsObserver {
  public:
   explicit TestWebContentsObserver(WebContents* contents)
-      : WebContentsObserver(contents) {
+      : WebContentsObserver(contents),
+        last_theme_color_(SK_ColorTRANSPARENT) {
   }
   ~TestWebContentsObserver() override {}
 
@@ -259,10 +261,16 @@ class TestWebContentsObserver : public WebContentsObserver {
     last_url_ = validated_url;
   }
 
+  void DidChangeThemeColor(SkColor theme_color) override {
+    last_theme_color_ = theme_color;
+  }
+
   const GURL& last_url() const { return last_url_; }
+  SkColor last_theme_color() const { return last_theme_color_; }
 
  private:
   GURL last_url_;
+  SkColor last_theme_color_;
 
   DISALLOW_COPY_AND_ASSIGN(TestWebContentsObserver);
 };
@@ -3116,4 +3124,40 @@ TEST_F(WebContentsImplTest, MediaPowerSaveBlocking) {
   EXPECT_FALSE(contents()->has_audio_power_save_blocker_for_testing());
 }
 
+TEST_F(WebContentsImplTest, ThemeColorChangeDependingOnFirstVisiblePaint) {
+  TestWebContentsObserver observer(contents());
+  TestRenderFrameHost* rfh = contents()->GetMainFrame();
+
+  SkColor transparent = SK_ColorTRANSPARENT;
+
+  EXPECT_EQ(transparent, contents()->GetThemeColor());
+  EXPECT_EQ(transparent, observer.last_theme_color());
+
+  // Theme color changes should not propagate past the WebContentsImpl before
+  // the first visually non-empty paint has occurred.
+  RenderViewHostTester::TestOnMessageReceived(
+      test_rvh(),
+      FrameHostMsg_DidChangeThemeColor(rfh->GetRoutingID(), SK_ColorRED));
+
+  EXPECT_EQ(SK_ColorRED, contents()->GetThemeColor());
+  EXPECT_EQ(transparent, observer.last_theme_color());
+
+  // Simulate that the first visually non-empty paint has occurred. This will
+  // propagate the current theme color to the delegates.
+  RenderViewHostTester::TestOnMessageReceived(
+      test_rvh(),
+      FrameHostMsg_DidFirstVisuallyNonEmptyPaint(rfh->GetRoutingID()));
+
+  EXPECT_EQ(SK_ColorRED, contents()->GetThemeColor());
+  EXPECT_EQ(SK_ColorRED, observer.last_theme_color());
+
+  // Additional changes made by the web contents should propagate as well.
+  RenderViewHostTester::TestOnMessageReceived(
+      test_rvh(),
+      FrameHostMsg_DidChangeThemeColor(rfh->GetRoutingID(), SK_ColorGREEN));
+
+  EXPECT_EQ(SK_ColorGREEN, contents()->GetThemeColor());
+  EXPECT_EQ(SK_ColorGREEN, observer.last_theme_color());
+}
+
 }  // namespace content