Deflake PrerenderExcessiveMemory, fix race

chrome/browser/prerender/prerender_contents.cc

 // Copyright (c) 2011 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 "chrome/browser/prerender/prerender_contents.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/process_util.h"
@@ skipping 154 matching lines @@
   DCHECK(profile_ != NULL);
   DCHECK(!prerendering_has_started_);
   DCHECK(prerender_contents_.get() == NULL);
 
   prerendering_has_started_ = true;
   TabContents* new_contents = new TabContents(profile_, NULL, MSG_ROUTING_NONE,
                                               NULL, NULL);
   prerender_contents_.reset(new TabContentsWrapper(new_contents));
   TabContentsObserver::Observe(new_contents);
 
-  gfx::Rect tab_bounds;
   if (source_render_view_host) {
     DCHECK(source_render_view_host->view() != NULL);
     TabContents* source_tc =
         source_render_view_host->delegate()->GetAsTabContents();
     if (source_tc) {
       // So that history merging will work, get the max page ID
       // of the old page as a starting id.
       starting_page_id_ = source_tc->GetMaxPageID();
 
       // Set the size of the new TC to that of the old TC.
-      source_tc->view()->GetContainerBounds(&tab_bounds);
+      source_tc->view()->GetContainerBounds(&tab_bounds_);
     }
   } else {
     int max_page_id = -1;
     // Get the largest page ID of all open tabs as a starting id.
     for (BrowserList::BrowserVector::const_iterator browser_iter =
             BrowserList::begin();
          browser_iter != BrowserList::end();
          ++browser_iter) {
       const Browser* browser = *browser_iter;
       int num_tabs = browser->tab_count();
       for (int tab_index = 0; tab_index < num_tabs; ++tab_index) {
         TabContents* tab_contents = browser->GetTabContentsAt(tab_index);
         if (tab_contents != NULL)
           max_page_id = std::max(max_page_id, tab_contents->GetMaxPageID());
       }
     }
     starting_page_id_ = max_page_id;
 
     // Try to get the active tab of the active browser and use that for tab
     // bounds. If the browser has never been active, we will fail to get a size
     // but we shouldn't be prerendering in that case anyway.
     Browser* active_browser = BrowserList::GetLastActive();
     if (active_browser) {
       TabContents* active_tab_contents = active_browser->GetTabContentsAt(
           active_browser->active_index());
-      active_tab_contents->view()->GetContainerBounds(&tab_bounds);
+      active_tab_contents->view()->GetContainerBounds(&tab_bounds_);
     }
   }
 
   // Add a safety margin of kPrerenderPageIdOffset to the starting page id (for
   // things such as redirects).
   if (starting_page_id_ < 0)
     starting_page_id_ = 0;
   starting_page_id_ += kPrerenderPageIdOffset;
   prerender_contents_->controller().set_max_restored_page_id(starting_page_id_);
 
   tab_contents_delegate_.reset(new TabContentsDelegateImpl(this));
   new_contents->set_delegate(tab_contents_delegate_.get());
 
-  // Set the size of the prerender TabContents.
-  prerender_contents_->view()->SizeContents(tab_bounds.size());
-
   // Register as an observer of the RenderViewHost so we get messages.
   render_view_host_observer_.reset(
       new PrerenderRenderViewHostObserver(this, render_view_host_mutable()));
 
   child_id_ = render_view_host()->process()->id();
   route_id_ = render_view_host()->routing_id();
 
   // Register this with the ResourceDispatcherHost as a prerender
   // RenderViewHost. This must be done before the Navigate message to catch all
   // resource requests, but as it is on the same thread as the Navigate message
@@ skipping 12 matching lines @@
   // APP_TERMINATING before non-OTR profiles are destroyed).
   // TODO(tburkard): figure out if this is needed.
   notification_registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
                               Source<Profile>(profile_));
 
   // Register to inform new RenderViews that we're prerendering.
   notification_registrar_.Add(
       this, content::NOTIFICATION_RENDER_VIEW_HOST_CREATED_FOR_TAB,
       Source<TabContents>(new_contents));
 
-  // Register to be told when the RenderView is ready, so we can hide it.
-  // It will automatically be set to visible when we resize it, otherwise.
-  notification_registrar_.Add(this,
-                              content::NOTIFICATION_TAB_CONTENTS_CONNECTED,
-                              Source<TabContents>(new_contents));
-
   // Register for redirect notifications sourced from |this|.
   notification_registrar_.Add(
       this, content::NOTIFICATION_RESOURCE_RECEIVED_REDIRECT,
       Source<RenderViewHostDelegate>(GetRenderViewHostDelegate()));
 
   // Register for new windows from any source.
   notification_registrar_.Add(
       this, content::NOTIFICATION_CREATING_NEW_WINDOW_CANCELLED,
       Source<TabContents>(new_contents));
 
@@ skipping 97 matching lines @@
 
         // When a new RenderView is created for a prerendering TabContents,
         // tell the new RenderView it's being used for prerendering before any
         // navigations occur.  Note that this is always triggered before the
         // first navigation, so there's no need to send the message just after
         // the TabContents is created.
         new_render_view_host->Send(
             new ChromeViewMsg_SetIsPrerendering(
                 new_render_view_host->routing_id(),
                 true));
+
+        // Set the size of the prerender TabContents.  This must be done after
+        // the RenderView has been created so that the RenderView will be
+        // informated promptly of the size change.

[cbentzel, 2011/08/29 12:26:18]

Nit: informed

+        prerender_contents_->view()->SizeContents(tab_bounds_.size());

[cbentzel, 2011/08/29 12:26:18]

Is the RenderWidgetHostView hidden at this point? It looks like this turns into
a no-op if done then, at least on Windows. Or does the Hide right below take
care of that?

[mmenke, 2011/08/29 12:30:35]

RenderWidgetHostViews start out as visible.  This results in the TabContents
window being resized, which also results in resizing the RenderViewHost and
sending the size message (Synchronously, at least on Windows, OSX, and non-Views
Linux).

Then it's hidden in the next line.  Still need to figure out why one of the
tests was failing on each of linux_views and linux_chromeos after I landed this.

+
+        // Hide the tab contents.  Must be done after setting the size, as
+        // resizing currently forces the RenderView to set itself as visible.
+        prerender_contents_->tab_contents()->HideContents();
       }
       break;
     }
 
-    case content::NOTIFICATION_TAB_CONTENTS_CONNECTED: {
-      if (prerender_contents_.get()) {
-        DCHECK_EQ(Source<TabContents>(source).ptr(),
-                  prerender_contents_->tab_contents());
-        // Set the new TabContents and its RenderViewHost as hidden, to reduce
-        // resource usage.  This can only be done after the size has been sent
-        // to the RenderView, which is why it's done here.
-        prerender_contents_->tab_contents()->HideContents();
-      }
-      return;
-    }
-
     case content::NOTIFICATION_CREATING_NEW_WINDOW_CANCELLED: {
       if (prerender_contents_.get()) {
         CHECK(Source<TabContents>(source).ptr() ==
               prerender_contents_->tab_contents());
         // Since we don't want to permit child windows that would have a
         // window.opener property, terminate prerendering.
         Destroy(FINAL_STATUS_CREATE_NEW_WINDOW);
       }
       break;
     }
@@ skipping 208 matching lines @@
 bool PrerenderContents::IsCrossSiteNavigationPending() const {
   if (!prerender_contents_.get() || !prerender_contents_->tab_contents())
     return false;
   const TabContents* tab_contents = prerender_contents_->tab_contents();
   return (tab_contents->GetSiteInstance() !=
           tab_contents->GetPendingSiteInstance());
 }
 
 
 }  // namespace prerender