Propagate view background color when changing visible web contents.

content/browser/web_contents/web_contents_impl.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/web_contents/web_contents_impl.h"
 
 #include <stddef.h>
 
 #include <cmath>
 #include <utility>
@@ skipping 3936 matching lines @@
   int type = is_loading ? NOTIFICATION_LOAD_START : NOTIFICATION_LOAD_STOP;
   NotificationDetails det = NotificationService::NoDetails();
   if (details)
       det = Details<LoadNotificationDetails>(details);
   NotificationService::current()->Notify(
       type, Source<NavigationController>(&controller_), det);
 }
 
 void WebContentsImpl::NotifyViewSwapped(RenderViewHost* old_host,
                                         RenderViewHost* new_host) {
+  if (old_host && new_host) {
+    RenderWidgetHostView* old_view = old_host->GetWidget()->GetView();
+    RenderWidgetHostView* new_view = new_host->GetWidget()->GetView();

[nasko, 2016/11/09 20:18:58]

I would put this code in WebContentsImpl::NotifyFrameSwapped, as we are moving
away from RenderViewHost as an object. Each RenderFrameHost can give you access
to its view through RFH::GetView() directly.
Also, it will be out-of-process iframes compatible, as in the future we will
also swap views on cross-process navigations in iframes. I don't know if the
same issue exists for subframes though.

[chrishtr, 2016/11/09 21:45:21]

Done.

+    if (old_view && new_view)
+      new_view->SetBackgroundColor(old_view->background_color());

[clamy, 2016/11/09 15:56:35]

Mmh I think this happens after we've swapped the view, so couldn't it
potentially lead to a flash still?

I remember the goal was to use WCO::ReadyToCommitNavigation so that you could
now about the commit and set the background before it happens. Since this
function now works, I'd suggest using it instead.

[nasko, 2016/11/09 20:18:58]

It happens in the same event loop as part of commit, but I don't know how the
background color interacts with threads and outputting graphics.
 
That might be a doable approach. If it is just setting the color, then it is ok,
but if it needs to do work that affects the output further, I think it needs to
wait for commit before outputting anything.

[chrishtr, 2016/11/09 21:45:21]

Since it's the same event loop, there is no flash.

+  }
   // After sending out a swap notification, we need to send a disconnect
   // notification so that clients that pick up a pointer to |this| can NULL the
   // pointer.  See Bug 1230284.
   notify_disconnection_ = true;
   for (auto& observer : observers_)
     observer.RenderViewHostChanged(old_host, new_host);
 
   // Ensure that the associated embedder gets cleared after a RenderViewHost
   // gets swapped, so we don't reuse the same embedder next time a
   // RenderViewHost is attached to this WebContents.
@@ skipping 1214 matching lines @@
   dialog_manager_ = dialog_manager;
 }
 
 void WebContentsImpl::RemoveBindingSet(const std::string& interface_name) {
   auto it = binding_sets_.find(interface_name);
   if (it != binding_sets_.end())
     binding_sets_.erase(it);
 }
 
 }  // namespace content