[content] Fix background color update handling in RWHVAura.

content/browser/renderer_host/render_widget_host_view_aura.cc

 // Copyright (c) 2012 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.
 
 #include "content/browser/renderer_host/render_widget_host_view_aura.h"
 
 #include <set>
 #include <utility>
 
 #include "base/auto_reset.h"
@@ skipping 383 matching lines @@
       legacy_render_widget_host_HWND_(nullptr),
       legacy_window_destroyed_(false),
       virtual_keyboard_requested_(false),
 #endif
       has_snapped_to_boundary_(false),
       is_guest_view_hack_(is_guest_view_hack),
       device_scale_factor_(0.0f),
       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);
 
   // We should start observing the TextInputManager for IME-related events as
   // well as monitoring its lifetime.
   if (GetTextInputManager())
     GetTextInputManager()->AddObserver(this);
 
   bool overscroll_enabled = base::CommandLine::ForCurrentProcess()->
@@ skipping 332 matching lines @@
 }
 
 gfx::Rect RenderWidgetHostViewAura::GetViewBounds() const {
   return window_->GetBoundsInScreen();
 }
 
 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);
 }
 
 bool RenderWidgetHostViewAura::IsMouseLocked() {
   return event_handler_->mouse_locked();
 }
 
 gfx::Size RenderWidgetHostViewAura::GetVisibleViewportSize() const {
   gfx::Rect requested_rect(GetRequestedRendererSize());
@@ skipping 140 matching lines @@
 
 void RenderWidgetHostViewAura::OnSwapCompositorFrame(
     uint32_t compositor_frame_sink_id,
     cc::CompositorFrame frame) {
   TRACE_EVENT0("content", "RenderWidgetHostViewAura::OnSwapCompositorFrame");
 
   // Override the background color to the current compositor background.
   // 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())
     return;
 
   cc::Selection<gfx::SelectionBound> selection = frame.metadata.selection;
   if (IsUseZoomForDSFEnabled()) {
     float viewportToDIPScale = 1.0f / current_device_scale_factor_;
     gfx::PointF start_edge_top = selection.start.edge_top();
     gfx::PointF start_edge_bottom = selection.start.edge_bottom();
@@ skipping 1441 matching lines @@
 
 void RenderWidgetHostViewAura::SetPopupChild(
     RenderWidgetHostViewAura* popup_child_host_view) {
   popup_child_host_view_ = popup_child_host_view;
   event_handler_->SetPopupChild(
       popup_child_host_view,
       popup_child_host_view ? popup_child_host_view->event_handler() : nullptr);
 }
 
 }  // namespace content