[content] Fix background color update handling in RWHVAura.

content/browser/renderer_host/render_widget_host_view_aura.cc

Index: content/browser/renderer_host/render_widget_host_view_aura.cc
diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc
index 010e0590b5352cbba30cf88ac2cd54b7284a91a5..f6d30329640b516090f471caa667ce5eabebdd5e 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura.cc
@@ -401,6 +401,7 @@ RenderWidgetHostViewAura::RenderWidgetHostViewAura(RenderWidgetHost* host,
       last_active_widget_process_id_(ChildProcessHost::kInvalidUniqueID),
       last_active_widget_routing_id_(MSG_ROUTING_NONE),
       event_handler_(new RenderWidgetHostViewEventHandler(host_, this, this)),
+      cached_background_color_(background_color_),
       weak_ptr_factory_(this) {
   if (!is_guest_view_hack_)
     host_->SetView(this);
@@ -753,8 +754,20 @@ void RenderWidgetHostViewAura::SetBackgroundColor(SkColor color) {
   if (color == background_color())
     return;
   RenderWidgetHostViewBase::SetBackgroundColor(color);
-  bool opaque = GetBackgroundOpaque();
-  host_->SetBackgroundOpaque(opaque);
+  host_->SetBackgroundOpaque(GetBackgroundOpaque());

[danakj, 2017/02/24 16:01:23]

I notice that other RenderWidgetHostViews do similar as here.
https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_wid...
etc..

Can we DCHECK that the color is fully opaque or transparent at each place that
does SetBackgroundOpaque? Actually, while we're at it, can we delete
SetBackgroundOpaque entirely and just check the color at each site? It doesn't
look like it needs to be part of the View API.

[Eric Seckler, 2017/02/27 13:46:26]

Done.

+
+  // The renderer will feed its color back to us with the first CompositorFrame.
+  // We short-cut here to show a sensible color before that happens.
+  UpdateBackgroundColorFromRenderer(color);
+}
+
+void RenderWidgetHostViewAura::UpdateBackgroundColorFromRenderer(
+    SkColor color) {
+  if (color == cached_background_color_)

[danakj, 2017/02/24 16:01:23]

I think this cached_background_color_ will cause a problem. The view's
background_color() won't reflect it, so when we navigate and such, we'll see the
wrong color there and it will cause flashing.

It seems like UpdateBackgroundColorFromRenderer wants to call
Base::SetBackgroundColor, which is a bit weird maybe. Or maybe not?

[danakj, 2017/02/24 16:04:08]

Oh, and at that point, the comment in the base class is wrong, it does accept
partial alpha if it comes from the renderer... which makes me feel like the
inheritance pattern here is being problematic. Maybe each subclass should
implement BackgroundColor() and we drop the impl of SetBackgroundColor and
background_color() from the base class?

[Eric Seckler, 2017/02/27 13:46:26]

Got rid of the base class implementations and updated derivations.

+    return;
+  cached_background_color_ = color;
+
+  bool opaque = SkColorGetA(color) == SK_AlphaOPAQUE;
   window_->layer()->SetFillsBoundsOpaquely(opaque);
   window_->layer()->SetColor(color);
 }
@@ -915,7 +928,8 @@ void RenderWidgetHostViewAura::OnSwapCompositorFrame(
   // This allows us to, when navigating to a new page, transfer this color to
   // that page. This allows us to pass this background color to new views on
   // navigation.
-  SetBackgroundColor(frame.metadata.root_background_color);
+  SkColor root_background_color = frame.metadata.root_background_color;
+  UpdateBackgroundColorFromRenderer(root_background_color);
 
   last_scroll_offset_ = frame.metadata.root_scroll_offset;
   if (frame.render_pass_list.empty())