Reduce likelihood of double-get of top-level prerendered resource.

When a link to a prerendered page is clicked, a
DidStartProvisionalLoadForFrame is sent, as well as a RequestResource
IPC for the top-level page, both from the referring page. The
RequestResource caused an additional GET of the top-level page, which
is frequently uncacheable.

To prevent this from happening, RequestResource's which _might_ match
a prerendered page URL are deferred. On the UI thread, the child_id
and route_id of the request are checked against active
RenderViewHost's. If there is no longer an active RenderViewHost
(which would happen if the prerendered page is swapped in), the
request is cancelled. Otherwise, it is resumed.

BUG=71089
TEST=Existing browser_tests, unit_tests --gtest_filter=*PrerenderTrackerUrls*, click on a link that is being prerendered and make sure that a request is not sent over the wire.


Review URL: http://codereview.chromium.org/7074001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@87258 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 300 insertions, 13 deletions

diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index 6337197..f490220 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -750,7 +750,8 @@ void BrowserProcessImpl::CreateResourceDispatcherHost() {
   resource_dispatcher_host_->Initialize();
 
   resource_dispatcher_host_observer_.reset(
-      new ChromeResourceDispatcherHostObserver(prerender_tracker()));
+      new ChromeResourceDispatcherHostObserver(resource_dispatcher_host_.get(),
+                                               prerender_tracker()));
   resource_dispatcher_host_->set_observer(
       resource_dispatcher_host_observer_.get());
 
diff --git a/chrome/browser/prerender/prerender_contents.cc b/chrome/browser/prerender/prerender_contents.cc
index 2480971..0efbc0c 100644
--- a/chrome/browser/prerender/prerender_contents.cc
+++ b/chrome/browser/prerender/prerender_contents.cc
@@ -280,6 +280,9 @@ PrerenderContents::~PrerenderContents() {
   // destroy it.
   if (prerender_contents_.get())
     delete ReleasePrerenderContents();
+
+  // The following URLs are no longer rendering.
+  prerender_tracker_->RemovePrerenderURLsOnUIThread(alias_urls_);
 }
 
 void PrerenderContents::Observe(NotificationType type,
@@ -406,6 +409,7 @@ bool PrerenderContents::AddAliasURL(const GURL& url) {
     return false;
   }
   alias_urls_.push_back(url);
+  prerender_tracker_->AddPrerenderURLOnUIThread(url);
   return true;
 }
 
diff --git a/chrome/browser/prerender/prerender_manager.cc b/chrome/browser/prerender/prerender_manager.cc
index a18ab64..58c8f76 100644
--- a/chrome/browser/prerender/prerender_manager.cc
+++ b/chrome/browser/prerender/prerender_manager.cc
@@ -785,6 +785,18 @@ void PrerenderManager::DeleteOldTabContents() {
   }
 }
 
+bool PrerenderManager::IsOldRenderViewHost(
+    const RenderViewHost* render_view_host) const {
+  for (std::list<TabContentsWrapper*>::const_iterator it =
+           old_tab_contents_list_.begin();
+       it != old_tab_contents_list_.end();
+       ++it) {
+    if ((*it)->tab_contents()->render_view_host() == render_view_host)
+      return true;
+  }
+  return false;
+}
+
 void PrerenderManager::PeriodicCleanup() {
   DCHECK(CalledOnValidThread());
   DeleteOldTabContents();
diff --git a/chrome/browser/prerender/prerender_manager.h b/chrome/browser/prerender/prerender_manager.h
index 153657d..f35aa18 100644
--- a/chrome/browser/prerender/prerender_manager.h
+++ b/chrome/browser/prerender/prerender_manager.h
@@ -169,6 +169,7 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
   void MarkTabContentsAsNotPrerendered(TabContents* tab_contents);
   bool IsTabContentsPrerendered(TabContents* tab_contents) const;
   bool WouldTabContentsBePrerendered(TabContents* tab_contents) const;
+  bool IsOldRenderViewHost(const RenderViewHost* render_view_host) const;
 
   // Records that some visible tab navigated (or was redirected) to the
   // provided URL.
diff --git a/chrome/browser/prerender/prerender_tracker.cc b/chrome/browser/prerender/prerender_tracker.cc
index 19de266..b209ae9 100644
--- a/chrome/browser/prerender/prerender_tracker.cc
+++ b/chrome/browser/prerender/prerender_tracker.cc
@@ -9,11 +9,127 @@
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/resource_context.h"
+#include "content/browser/renderer_host/render_view_host.h"
+#include "content/browser/renderer_host/resource_dispatcher_host.h"
 #include "content/common/resource_messages.h"
 #include "net/base/load_flags.h"
 
 namespace prerender {
 
+namespace {
+
+void CancelDeferredRequestOnIOThread(
+    ResourceDispatcherHost* resource_dispatcher_host,
+    int child_id, int request_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  resource_dispatcher_host->CancelRequest(child_id, request_id, false);
+}
+
+void StartDeferredRequestOnIOThread(
+    ResourceDispatcherHost* resource_dispatcher_host,
+    int child_id, int request_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  resource_dispatcher_host->StartDeferredRequest(child_id, request_id);
+}
+
+bool ShouldCancelRequest(
+    const base::WeakPtr<PrerenderManager>& prerender_manager_ptr,
+    int child_id,
+    int route_id) {
+  // Check if the RenderViewHost associated with (child_id, route_id) no
+  // longer exists, or has already been swapped out for a prerendered page.
+  // If that happens, then we do not want to issue the request which originated
+  // from it. Otherwise, keep it going.
+  // The RenderViewHost may exist for a couple of different reasons: such as
+  // being an XHR from the originating page, being included as an iframe,
+  // being requested as a favicon or stylesheet, and many other corner cases.
+  RenderViewHost* render_view_host =
+      RenderViewHost::FromID(child_id, route_id);
+  if (!render_view_host)
+    return true;
+  PrerenderManager* prerender_manager = prerender_manager_ptr.get();
+  return (prerender_manager &&
+          prerender_manager->IsOldRenderViewHost(render_view_host));
+}
+
+void HandleDelayedRequestOnUIThread(
+    const base::WeakPtr<PrerenderManager>& prerender_manager,
+    int child_id,
+    int route_id,
+    int request_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  ResourceDispatcherHost* resource_dispatcher_host =
+      g_browser_process->resource_dispatcher_host();
+  CHECK(resource_dispatcher_host);
+  if (ShouldCancelRequest(prerender_manager, child_id, route_id)) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        NewRunnableFunction(&CancelDeferredRequestOnIOThread,
+                            resource_dispatcher_host, child_id, request_id));
+  } else {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        NewRunnableFunction(&StartDeferredRequestOnIOThread,
+                            resource_dispatcher_host, child_id, request_id));
+  }
+}
+
+void AddURL(const GURL& url, URLCounter* counter) {
+  DCHECK(counter);
+  counter->AddURL(url);
+}
+
+void RemoveURLs(const std::vector<GURL>& urls, URLCounter* counter) {
+  DCHECK(counter);
+  counter->RemoveURLs(urls);
+}
+
+}  // namespace
+
+URLCounter::URLCounter() {
+  // URLCounter is currently constructed on the UI thread but
+  // accessed on the IO thread.
+  DetachFromThread();
+}
+
+URLCounter::~URLCounter() {
+  // URLCounter is currently destructed on the UI thread but
+  // accessed on the IO thread.
+  DetachFromThread();
+}
+
+bool URLCounter::MatchesURL(const GURL& url) const {
+  DCHECK(CalledOnValidThread());
+  URLCountMap::const_iterator it = url_count_map_.find(url);
+  if (it == url_count_map_.end())
+    return false;
+  DCHECK(it->second > 0);
+  return true;
+}
+
+void URLCounter::AddURL(const GURL& url) {
+  DCHECK(CalledOnValidThread());
+  URLCountMap::iterator it = url_count_map_.find(url);
+  if (it == url_count_map_.end())
+    url_count_map_[url] = 1;
+  else
+    it->second++;
+}
+
+void URLCounter::RemoveURLs(const std::vector<GURL>& urls) {
+  DCHECK(CalledOnValidThread());
+  for (std::vector<GURL>::const_iterator it = urls.begin();
+       it != urls.end();
+       ++it) {
+    URLCountMap::iterator map_entry = url_count_map_.find(*it);
+    DCHECK(url_count_map_.end() != map_entry);
+    DCHECK(map_entry->second > 0);
+    --map_entry->second;
+    if (map_entry->second == 0)
+      url_count_map_.erase(map_entry);
+  }
+}
+
 struct RenderViewInfo {
   explicit RenderViewInfo(PrerenderManager* prerender_manager)
       : final_status(FINAL_STATUS_MAX),
@@ -62,6 +178,41 @@ bool PrerenderTracker::TryCancelOnIOThread(
   return TryCancel(child_id, route_id, final_status);
 }
 
+bool PrerenderTracker::PotentiallyDelayRequestOnIOThread(
+    const GURL& gurl,
+    const base::WeakPtr<PrerenderManager>& prerender_manager,
+    int process_id,
+    int route_id,
+    int request_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (!url_counter_.MatchesURL(gurl))
+    return false;
+  BrowserThread::PostTask(
+      BrowserThread::UI,
+      FROM_HERE,
+      NewRunnableFunction(&HandleDelayedRequestOnUIThread,
+                          prerender_manager,
+                          process_id,
+                          route_id,
+                          request_id));
+  return true;
+}
+
+void PrerenderTracker::AddPrerenderURLOnUIThread(const GURL& url) {
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      NewRunnableFunction(&AddURL, url, &url_counter_));
+}
+
+void PrerenderTracker::RemovePrerenderURLsOnUIThread(
+    const std::vector<GURL>& urls) {
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      NewRunnableFunction(&RemoveURLs, urls, &url_counter_));
+}
+
 bool PrerenderTracker::GetFinalStatus(int child_id, int route_id,
                                       FinalStatus* final_status) const {
   ChildRouteIdPair child_route_id_pair(child_id, route_id);
@@ -181,17 +332,20 @@ void PrerenderTracker::RemovePrerenderOnIOThread(
 }
 
 // static
+PrerenderTracker* PrerenderTracker::GetDefault() {
+  return g_browser_process->prerender_tracker();
+}
+
+// static
 void PrerenderTracker::AddPrerenderOnIOThreadTask(
     const ChildRouteIdPair& child_route_id_pair) {
-  g_browser_process->prerender_tracker()->AddPrerenderOnIOThread(
-      child_route_id_pair);
+  GetDefault()->AddPrerenderOnIOThread(child_route_id_pair);
 }
 
 // static
 void PrerenderTracker::RemovePrerenderOnIOThreadTask(
     const ChildRouteIdPair& child_route_id_pair) {
-  g_browser_process->prerender_tracker()->RemovePrerenderOnIOThread(
-      child_route_id_pair);
+  GetDefault()->RemovePrerenderOnIOThread(child_route_id_pair);
 }
 
 }  // namespace prerender
diff --git a/chrome/browser/prerender/prerender_tracker.h b/chrome/browser/prerender/prerender_tracker.h
index 3167fa1..d8be929 100644
--- a/chrome/browser/prerender/prerender_tracker.h
+++ b/chrome/browser/prerender/prerender_tracker.h
@@ -8,16 +8,40 @@
 
 #include <map>
 #include <set>
+#include <vector>
 
 #include "base/gtest_prod_util.h"
+#include "base/memory/weak_ptr.h"
 #include "base/synchronization/lock.h"
+#include "base/threading/non_thread_safe.h"
 #include "chrome/browser/prerender/prerender_final_status.h"
+#include "googleurl/src/gurl.h"
 
 namespace prerender {
 
 class PrerenderManager;
 struct RenderViewInfo;
 
+// An URLCounter keeps track of the number of occurrences of a prerendered URL.
+class URLCounter : public base::NonThreadSafe {
+ public:
+  URLCounter();
+  ~URLCounter();
+
+  // Determines whether the URL is contained in the set.
+  bool MatchesURL(const GURL& url) const;
+
+  // Adds a URL to the set.
+  void AddURL(const GURL& url);
+
+  // Removes a number of URLs from the set.
+  void RemoveURLs(const std::vector<GURL>& urls);
+
+ private:
+  typedef std::map<GURL, int> URLCountMap;
+  URLCountMap url_count_map_;
+};
+
 // PrerenderTracker is responsible for keeping track of all prerendering
 // RenderViews and their statuses.  Its list is guaranteed to be up to date
 // and can be modified on any thread.
@@ -53,6 +77,18 @@ class PrerenderTracker {
   bool TryCancelOnIOThread(int child_id, int route_id,
                            FinalStatus final_status);
 
+  // Potentially delay a resource request on the IO thread to prevent a double
+  // get.
+  bool PotentiallyDelayRequestOnIOThread(
+      const GURL& gurl,
+      const base::WeakPtr<PrerenderManager>& prerender_manager,
+      int child_id,
+      int route_id,
+      int request_id);
+
+  void AddPrerenderURLOnUIThread(const GURL& url);
+  void RemovePrerenderURLsOnUIThread(const std::vector<GURL>& urls);
+
   // Gets the FinalStatus of the specified prerendered RenderView.  Returns
   // |true| and sets |final_status| to the status of the RenderView if it
   // is found, returns false otherwise.
@@ -66,7 +102,6 @@ class PrerenderTracker {
 
  private:
   friend class PrerenderContents;
-
   FRIEND_TEST_ALL_PREFIXES(PrerenderTrackerTest, PrerenderTrackerNull);
   FRIEND_TEST_ALL_PREFIXES(PrerenderTrackerTest, PrerenderTrackerUsed);
   FRIEND_TEST_ALL_PREFIXES(PrerenderTrackerTest, PrerenderTrackerCancelled);
@@ -75,7 +110,6 @@ class PrerenderTracker {
   FRIEND_TEST_ALL_PREFIXES(PrerenderTrackerTest, PrerenderTrackerMultiple);
 
   typedef std::pair<int, int> ChildRouteIdPair;
-
   // Map of child/route id pairs to final statuses.
   typedef std::map<ChildRouteIdPair, RenderViewInfo> FinalStatusMap;
   // Set of child/route id pairs that may be prerendering.
@@ -118,6 +152,8 @@ class PrerenderTracker {
   static void RemovePrerenderOnIOThreadTask(
       const ChildRouteIdPair& child_route_id_pair);
 
+  static PrerenderTracker* GetDefault();
+
   // |final_status_map_lock_| protects access to |final_status_map_|.
   mutable base::Lock final_status_map_lock_;
   // Map containing child/route id pairs and their final statuses.  Must only be
@@ -130,6 +166,10 @@ class PrerenderTracker {
   // used to prevent locking when not needed.
   PossiblyPrerenderingChildRouteIdPairs possibly_prerendering_io_thread_set_;
 
+  // |url_counter_| keeps track of the top-level URLs which are being
+  // prerendered. It must only be accessed on the IO thread.
+  URLCounter url_counter_;
+
   DISALLOW_COPY_AND_ASSIGN(PrerenderTracker);
 };
 
diff --git a/chrome/browser/prerender/prerender_tracker_unittest.cc b/chrome/browser/prerender/prerender_tracker_unittest.cc
index b89ddbd..2518b691 100644
--- a/chrome/browser/prerender/prerender_tracker_unittest.cc
+++ b/chrome/browser/prerender/prerender_tracker_unittest.cc
@@ -294,4 +294,45 @@ TEST_F(PrerenderTrackerTest, PrerenderTrackerMultiple) {
   EXPECT_FALSE(prerender_tracker()->IsPrerenderingOnIOThread(1, 2));
 }
 
+// Tests that Prerender tracking works correctly
+TEST_F(PrerenderTrackerTest, URLCounter) {
+  URLCounter url_counter;
+  GURL example_url("http://www.example.com");
+  GURL gmail_url("https://www.gmail.com");
+
+  EXPECT_FALSE(url_counter.MatchesURL(example_url));
+  EXPECT_FALSE(url_counter.MatchesURL(gmail_url));
+
+  url_counter.AddURL(example_url);
+  url_counter.AddURL(example_url);
+  url_counter.AddURL(gmail_url);
+  EXPECT_TRUE(url_counter.MatchesURL(example_url));
+  EXPECT_TRUE(url_counter.MatchesURL(gmail_url));
+
+  std::vector<GURL> remove_urls;
+  remove_urls.push_back(example_url);
+  remove_urls.push_back(gmail_url);
+  url_counter.RemoveURLs(remove_urls);
+  EXPECT_TRUE(url_counter.MatchesURL(example_url));
+  EXPECT_FALSE(url_counter.MatchesURL(gmail_url));
+
+  remove_urls.clear();
+  remove_urls.push_back(example_url);
+  url_counter.RemoveURLs(remove_urls);
+  EXPECT_FALSE(url_counter.MatchesURL(example_url));
+  EXPECT_FALSE(url_counter.MatchesURL(gmail_url));
+
+  url_counter.AddURL(example_url);
+  url_counter.AddURL(example_url);
+  EXPECT_TRUE(url_counter.MatchesURL(example_url));
+  EXPECT_FALSE(url_counter.MatchesURL(gmail_url));
+
+  remove_urls.clear();
+  remove_urls.push_back(example_url);
+  remove_urls.push_back(example_url);
+  url_counter.RemoveURLs(remove_urls);
+  EXPECT_FALSE(url_counter.MatchesURL(example_url));
+  EXPECT_FALSE(url_counter.MatchesURL(gmail_url));
+}
+
 }  // namespace prerender
diff --git a/chrome/browser/renderer_host/chrome_resource_dispatcher_host_observer.cc b/chrome/browser/renderer_host/chrome_resource_dispatcher_host_observer.cc
index ed38156..73dbf0d 100644
--- a/chrome/browser/renderer_host/chrome_resource_dispatcher_host_observer.cc
+++ b/chrome/browser/renderer_host/chrome_resource_dispatcher_host_observer.cc
@@ -9,12 +9,15 @@
 #include "chrome/browser/prerender/prerender_tracker.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/resource_context.h"
+#include "content/browser/renderer_host/resource_dispatcher_host_request_info.h"
 #include "content/common/resource_messages.h"
 #include "net/base/load_flags.h"
 
 ChromeResourceDispatcherHostObserver::ChromeResourceDispatcherHostObserver(
+    ResourceDispatcherHost* resource_dispatcher_host,
     prerender::PrerenderTracker* prerender_tracker)
-    : prerender_tracker_(prerender_tracker) {
+    : resource_dispatcher_host_(resource_dispatcher_host),
+      prerender_tracker_(prerender_tracker) {
 }
 
 ChromeResourceDispatcherHostObserver::~ChromeResourceDispatcherHostObserver() {
@@ -67,6 +70,16 @@ bool ChromeResourceDispatcherHostObserver::ShouldBeginRequest(
   return true;
 }
 
+bool ChromeResourceDispatcherHostObserver::ShouldDeferStart(
+    net::URLRequest* request,
+    const content::ResourceContext& resource_context) {
+  ResourceDispatcherHostRequestInfo* info =
+      resource_dispatcher_host_->InfoForRequest(request);
+  return prerender_tracker_->PotentiallyDelayRequestOnIOThread(
+      request->url(), resource_context.prerender_manager(),
+      info->child_id(), info->route_id(), info->request_id());
+}
+
 void ChromeResourceDispatcherHostObserver::MutateLoadFlags(int child_id,
                                                            int route_id,
                                                            int* load_flags) {
diff --git a/chrome/browser/renderer_host/chrome_resource_dispatcher_host_observer.h b/chrome/browser/renderer_host/chrome_resource_dispatcher_host_observer.h
index e2345cd..e336e89 100644
--- a/chrome/browser/renderer_host/chrome_resource_dispatcher_host_observer.h
+++ b/chrome/browser/renderer_host/chrome_resource_dispatcher_host_observer.h
@@ -18,9 +18,11 @@ class ChromeResourceDispatcherHostObserver
     : public ResourceDispatcherHost::Observer {
  public:
   // This class does not take ownership of the tracker but merely holds a
-  // reference to it to avoid accessing g_browser_process. The PrerenderTracker
-  // will be destroyed after the observer.
-  explicit ChromeResourceDispatcherHostObserver(
+  // reference to it to avoid accessing g_browser_process.
+  // Both |resource_dispatcher_host| and |prerender_tracker| must outlive
+  // |this|.
+  ChromeResourceDispatcherHostObserver(
+      ResourceDispatcherHost* resource_dispatcher_host,
       prerender::PrerenderTracker* prerender_tracker);
   virtual ~ChromeResourceDispatcherHostObserver();
 
@@ -31,6 +33,10 @@ class ChromeResourceDispatcherHostObserver
       const content::ResourceContext& resource_context,
       const GURL& referrer) OVERRIDE;
 
+  virtual bool ShouldDeferStart(
+      net::URLRequest* request,
+      const content::ResourceContext& resource_context) OVERRIDE;
+
   virtual void MutateLoadFlags(int child_id, int route_id,
                                int* load_flags) OVERRIDE;
 
@@ -42,6 +48,7 @@ class ChromeResourceDispatcherHostObserver
                                  net::AuthChallengeInfo* auth_info) OVERRIDE;
 
  private:
+  ResourceDispatcherHost* resource_dispatcher_host_;
   prerender::PrerenderTracker* prerender_tracker_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeResourceDispatcherHostObserver);
diff --git a/content/browser/renderer_host/resource_dispatcher_host.cc b/content/browser/renderer_host/resource_dispatcher_host.cc
index ce8fb05..b7512ae 100644
--- a/content/browser/renderer_host/resource_dispatcher_host.cc
+++ b/content/browser/renderer_host/resource_dispatcher_host.cc
@@ -1387,9 +1387,15 @@ void ResourceDispatcherHost::BeginRequestInternal(net::URLRequest* request) {
     return;
   }
 
-  if (!defer_start) {
-    InsertIntoResourceQueue(request, *info);
+  // TODO(cbentzel): Should we isolate this to resource handlers instead of
+  // adding an interface to the observer?
+  if (!defer_start && observer_ && filter_) {
+    defer_start = observer_->ShouldDeferStart(request,
+                                              filter_->resource_context());
   }
+
+  if (!defer_start)
+    InsertIntoResourceQueue(request, *info);
 }
 
 void ResourceDispatcherHost::InsertIntoResourceQueue(
diff --git a/content/browser/renderer_host/resource_dispatcher_host.h b/content/browser/renderer_host/resource_dispatcher_host.h
index 1d3a0dc..60d4193 100644
--- a/content/browser/renderer_host/resource_dispatcher_host.h
+++ b/content/browser/renderer_host/resource_dispatcher_host.h
@@ -73,6 +73,14 @@ class ResourceDispatcherHost : public net::URLRequest::Delegate {
     virtual void MutateLoadFlags(int child_id, int route_id,
                                  int* load_flags) = 0;
 
+    // Called to determine whether a request's start should be deferred. This
+    // is only called if the ResourceHandler associated with the request does
+    // not ask for a deferral. A return value of true will defer the start of
+    // the request, false will continue the request.
+    virtual bool ShouldDeferStart(
+        net::URLRequest* request,
+        const content::ResourceContext& resource_context) = 0;
+
     // Called when an SSL Client Certificate is requested. If false is returned,
     // the request is canceled. Otherwise, the certificate is chosen.
     virtual bool AcceptSSLClientCertificateRequest(