Start caching the theme color in web_contents

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index d47a6ef362c8157626ee30e86106e2604b0cfb80..7a5f0bbd293201abc8b4956c0bb7fea41815823f 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -128,6 +128,8 @@ const double kMinimumLoadingProgress = 0.1;
 
 const char kDotGoogleDotCom[] = ".google.com";
 
+const SkColor kDefaultThemeColor = 0xFFF2F2F2;

[no sievers, 2015/03/05 20:18:41]

It would be nice if this could be set by the application.

What's actually easier is to not worry about the default color here at all, but
rather make it 'SkColor* WebContents::GetThemeColor()' which returns NULL for
none/default (for example by having scoped_ptr<SkColor>
WebContentsImpl::theme_color_).

Then you could either plumb this up to Java by doing the same
(android.graphics.Color WebContents.getThemeColor() which could return null), or
you split it into two APIs in web_contents_android.cc
(isThemeColorValid()/getThemeColor()).

[Yusuf, 2015/03/05 22:05:15]

Moved to setting it to transparent and handling the rest on the java side. We
can not set the omnibox to transparent anyway, so I think using
Sk_ColorTRANSPARENT is a good solution.

+
 #if defined(OS_ANDROID)
 const char kWebContentsAndroidKey[] = "web_contents_android";
 #endif  // OS_ANDROID
@@ -341,6 +343,7 @@ WebContentsImpl::WebContentsImpl(BrowserContext* browser_context,
       upload_position_(0),
       displayed_insecure_content_(false),
       has_accessed_initial_document_(false),
+      theme_color_(kDefaultThemeColor),
       capturer_count_(0),
       should_normally_be_visible_(true),
       is_being_destroyed_(false),
@@ -720,6 +723,10 @@ WebContentsView* WebContentsImpl::GetView() const {
   return view_.get();
 }
 
+SkColor WebContentsImpl::GetThemeColor() const {
+  return theme_color_;
+}
+
 void WebContentsImpl::SetAccessibilityMode(AccessibilityMode mode) {
   if (mode == accessibility_mode_)
     return;
@@ -2717,6 +2724,9 @@ void WebContentsImpl::DidNavigateMainFramePostCommit(
         static_cast<RenderWidgetHostViewBase*>(GetRenderWidgetHostView());
     if (rwhvb)
       rwhvb->OnDidNavigateMainFrameToNewPage();
+
+    // Reset theme color on navigation to new page.
+    theme_color_ = kDefaultThemeColor;
   }
 
   if (!details.is_in_page) {
@@ -2769,8 +2779,9 @@ bool WebContentsImpl::CanOverscrollContent() const {
 }
 
 void WebContentsImpl::OnThemeColorChanged(SkColor theme_color) {
-  FOR_EACH_OBSERVER(WebContentsObserver, observers_,
-                    DidChangeThemeColor(theme_color));
+  // Update the theme color. This is to be published to observers on visually
+  // non empty paint.
+  theme_color_ = theme_color;
 }
 
 void WebContentsImpl::OnDidLoadResourceFromMemoryCache(
@@ -3264,6 +3275,10 @@ void WebContentsImpl::OnMediaPausedNotification(int64 player_cookie) {
 void WebContentsImpl::OnFirstVisuallyNonEmptyPaint() {
   FOR_EACH_OBSERVER(WebContentsObserver, observers_,
                     DidFirstVisuallyNonEmptyPaint());
+
+  // Theme color should have updated by now if there was one.
+  FOR_EACH_OBSERVER(WebContentsObserver, observers_,
+                    DidChangeThemeColor(theme_color_));

[no sievers, 2015/03/05 20:18:41]

Should we cache the last sent color and only notify if it really changed?

[Yusuf, 2015/03/05 22:05:15]

Done.

 }
 
 void WebContentsImpl::DidChangeVisibleSSLState() {