Only commit cookie changes in prerenders after a prerender is shown

Will create a PrerenderCookieStore for each prerender, retaining all cookie
operations of a prerender until the prerender is shown to the user.
Forces prerenders to be in a new render process by themselves for this to work.
BUG=371003
R=creis@chromium.org, davidben@chromium.org, erikwright@chromium.org, jam@chromium.org, jochen@chromium.org

Review URL: https://codereview.chromium.org/233353003

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

chrome/android/java/src/org/chromium/chrome/browser/prerender/ExternalPrerenderHandler.java

@@ -38,12 +38,18 @@ public static boolean hasPrerenderedUrl(Profile profile, String url, long webCon
         return nativeHasPrerenderedUrl(profile, url, webContentsPtr);
     }
 
+    public static boolean hasCookieStoreLoaded(Profile profile)  {
+        return nativeHasCookieStoreLoaded(profile);
+    }
+
     private static native long nativeInit();
     private static native boolean nativeAddPrerender(
             long nativeExternalPrerenderHandlerAndroid, Profile profile,
             long webContentsPtr, String url, String referrer, int width, int height);
     private static native boolean nativeHasPrerenderedUrl(
             Profile profile, String url, long webContentsPtr);
+    private static native boolean nativeHasCookieStoreLoaded(
+            Profile profile);
     private static native void nativeCancelCurrentPrerender(
             long nativeExternalPrerenderHandlerAndroid);
 }

chrome/android/javatests/src/org/chromium/chrome/browser/prerender/ExternalPrerenderRequestTest.java

@@ -21,11 +21,14 @@
  * Tests for adding and removing prerenders using the {@link ExternalPrerenderHandler}
  */
 public class ExternalPrerenderRequestTest extends ChromeShellTestBase {
+    private static final String HOMEPAGE_URL =
+            TestHttpServerClient.getUrl("chrome/test/data/android/prerender/homepage.html");
     private static final String GOOGLE_URL =
             TestHttpServerClient.getUrl("chrome/test/data/android/prerender/google.html");
     private static final String YOUTUBE_URL =
             TestHttpServerClient.getUrl("chrome/test/data/android/prerender/youtube.html");
     private static final int PRERENDER_DELAY_MS = 500;
+    private static final int CHECK_COOKIE_STORE_FREQUENCY_MS = 200;
 
     private ExternalPrerenderHandler mHandler;
     private Profile mProfile;
@@ -34,7 +37,8 @@ public class ExternalPrerenderRequestTest extends ChromeShellTestBase {
     public void setUp() throws Exception {
         super.setUp();
         clearAppData();
-        launchChromeShellWithBlankPage();
+        // Launch with a non-blank homepage, to trigger cookie store loading.
+        launchChromeShellWithUrl(HOMEPAGE_URL);
         assertTrue(waitForActiveShellToBeDoneLoading());
         mHandler = new ExternalPrerenderHandler();
         final Callable<Profile> profileCallable = new Callable<Profile>() {
@@ -44,6 +48,8 @@ public Profile call() throws Exception {
             }
         };
         mProfile = ThreadUtils.runOnUiThreadBlocking(profileCallable);
+        while (!ExternalPrerenderHandler.hasCookieStoreLoaded(mProfile))
+            Thread.sleep(CHECK_COOKIE_STORE_FREQUENCY_MS);
     }
 
     @MediumTest

chrome/browser/chrome_content_browser_client.cc

@@ -630,7 +630,8 @@ float GetDeviceScaleAdjustment() {
 
 namespace chrome {
 
-ChromeContentBrowserClient::ChromeContentBrowserClient() {
+ChromeContentBrowserClient::ChromeContentBrowserClient()
+    : prerender_tracker_(NULL) {
 #if defined(ENABLE_PLUGINS)
   for (size_t i = 0; i < arraysize(kPredefinedAllowedFileHandleOrigins); ++i)
     allowed_file_handle_origins_.insert(kPredefinedAllowedFileHandleOrigins[i]);
@@ -1214,6 +1215,24 @@ bool ChromeContentBrowserClient::IsSuitableHost(
       privilege_required;
 }
 
+bool ChromeContentBrowserClient::MayReuseHost(
+    content::RenderProcessHost* process_host) {
+  // If there is currently a prerender in progress for the host provided,
+  // it may not be shared. We require prerenders to be by themselves in a
+  // separate process, so that we can monitor their resource usage, and so that
+  // we can track the cookies that they change.
+  Profile* profile = Profile::FromBrowserContext(
+      process_host->GetBrowserContext());
+  prerender::PrerenderManager* prerender_manager =
+      prerender::PrerenderManagerFactory::GetForProfile(profile);
+  if (prerender_manager &&
+      prerender_manager->IsProcessPrerendering(process_host)) {
+    return false;
+  }
+
+  return true;
+}
+
 // This function is trying to limit the amount of processes used by extensions
 // with background pages. It uses a globally set percentage of processes to
 // run such extensions and if the limit is exceeded, it returns true, to
@@ -1765,6 +1784,13 @@ bool ChromeContentBrowserClient::AllowSetCookie(
   CookieSettings* cookie_settings = io_data->GetCookieSettings();
   bool allow = cookie_settings->IsSettingCookieAllowed(url, first_party);
 
+  if (prerender_tracker_) {
+    prerender_tracker_->OnCookieChangedForURL(
+        render_process_id,
+        context->GetRequestContext()->cookie_store()->GetCookieMonster(),
+        url);
+  }
+
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&TabSpecificContentSettings::CookieChanged, render_process_id,
@@ -2211,6 +2237,8 @@ std::string ChromeContentBrowserClient::GetWorkerProcessTitle(
 }
 
 void ChromeContentBrowserClient::ResourceDispatcherHostCreated() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  prerender_tracker_ = g_browser_process->prerender_tracker();
   return g_browser_process->ResourceDispatcherHostCreated();
 }
 
@@ -2698,6 +2726,16 @@ bool ChromeContentBrowserClient::IsPluginAllowedToUseDevChannelAPIs() {
 #endif
 }
 
+net::CookieStore*
+ChromeContentBrowserClient::OverrideCookieStoreForRenderProcess(
+    int render_process_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (!prerender_tracker_)
+    return NULL;
+  return prerender_tracker_->
+      GetPrerenderCookieStoreForRenderProcess(render_process_id);
+}
+
 #if defined(ENABLE_WEBRTC)
 void ChromeContentBrowserClient::MaybeCopyDisableWebRtcEncryptionSwitch(
     CommandLine* to_command_line,

chrome/browser/chrome_content_browser_client.h

@@ -31,6 +31,10 @@ namespace extensions {
 class BrowserPermissionsPolicyDelegate;
 }
 
+namespace prerender {
+class PrerenderTracker;
+}
+
 namespace user_prefs {
 class PrefRegistrySyncable;
 }
@@ -102,6 +106,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
                                   const GURL& url) OVERRIDE;
   virtual bool IsSuitableHost(content::RenderProcessHost* process_host,
                               const GURL& site_url) OVERRIDE;
+  virtual bool MayReuseHost(content::RenderProcessHost* process_host) OVERRIDE;
   virtual bool ShouldTryToUseExistingProcessHost(
       content::BrowserContext* browser_context, const GURL& url) OVERRIDE;
   virtual void SiteInstanceGotProcess(
@@ -278,6 +283,9 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
 
   virtual bool IsPluginAllowedToUseDevChannelAPIs() OVERRIDE;
 
+  virtual net::CookieStore* OverrideCookieStoreForRenderProcess(
+      int render_process_id) OVERRIDE;
+
  private:
 #if defined(ENABLE_WEBRTC)
   // Copies disable WebRTC encryption switch depending on the channel.
@@ -296,6 +304,14 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
   scoped_ptr<extensions::BrowserPermissionsPolicyDelegate>
       permissions_policy_delegate_;
 
+  // The prerender tracker used to determine whether a render process is used
+  // for prerendering and an override cookie store must be provided.
+  // This needs to be kept as a member rather than just looked up from
+  // the profile due to initialization ordering, as well as due to threading.
+  // It is initialized on the UI thread when the ResoureDispatcherHost is
+  // created. It is used only the IO thread.
+  prerender::PrerenderTracker* prerender_tracker_;
+
   friend class DisableWebRtcEncryptionFlagTest;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeContentBrowserClient);

chrome/browser/extensions/activity_log/activity_log_browsertest.cc

@@ -12,7 +12,10 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
+#include "content/public/browser/storage_partition.h"
 #include "content/public/test/browser_test_utils.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
@@ -99,6 +102,13 @@ IN_PROC_BROWSER_TEST_F(ActivityLogPrerenderTest, TestScriptInjected) {
       "http://www.google.com.bo:%d/test.html",
       port));
 
+  if (!prerender_manager->cookie_store_loaded()) {
+    base::RunLoop loop;
+    prerender_manager->set_on_cookie_store_loaded_cb_for_testing(
+        loop.QuitClosure());
+    loop.Run();
+  }
+
   const gfx::Size kSize(640, 480);
   scoped_ptr<prerender::PrerenderHandle> prerender_handle(
       prerender_manager->AddPrerenderFromLocalPredictor(

chrome/browser/extensions/activity_log/activity_log_unittest.cc

@@ -244,6 +244,8 @@ TEST_F(ActivityLogTest, LogPrerender) {
       prerender::PrerenderManagerFactory::GetForProfile(
           Profile::FromBrowserContext(profile()));
 
+  prerender_manager->OnCookieStoreLoaded();
+
   const gfx::Size kSize(640, 480);
   scoped_ptr<prerender::PrerenderHandle> prerender_handle(
       prerender_manager->AddPrerenderFromLocalPredictor(

chrome/browser/net/chrome_network_delegate.cc

@@ -29,6 +29,7 @@
 #include "chrome/browser/net/client_hints.h"
 #include "chrome/browser/net/connect_interceptor.h"
 #include "chrome/browser/performance_monitor/performance_monitor.h"
+#include "chrome/browser/prerender/prerender_tracker.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/task_manager/task_manager.h"
 #include "chrome/common/pref_names.h"
@@ -52,6 +53,7 @@
 #include "net/http/http_response_headers.h"
 #include "net/socket_stream/socket_stream.h"
 #include "net/url_request/url_request.h"
+#include "net/url_request/url_request_context.h"
 
 #if defined(OS_CHROMEOS)
 #include "base/command_line.h"
@@ -358,7 +360,8 @@ ChromeNetworkDelegate::ChromeNetworkDelegate(
       domain_reliability_monitor_(NULL),
       received_content_length_(0),
       original_content_length_(0),
-      first_request_(true) {
+      first_request_(true),
+      prerender_tracker_(NULL) {
   DCHECK(event_router);
   DCHECK(enable_referrers);
 }
@@ -742,6 +745,13 @@ bool ChromeNetworkDelegate::OnCanSetCookie(const net::URLRequest& request,
                    cookie_line, *options, !allow));
   }
 
+  if (prerender_tracker_) {
+    prerender_tracker_->OnCookieChangedForURL(
+        render_process_id,
+        request.context()->cookie_store()->GetCookieMonster(),
+        request.url());
+  }
+
   return allow;
 }
 

chrome/browser/net/chrome_network_delegate.h

@@ -49,6 +49,10 @@ namespace policy {
 class URLBlacklistManager;
 }
 
+namespace prerender {
+class PrerenderTracker;
+}
+
 // ChromeNetworkDelegate is the central point from within the chrome code to
 // add hooks into the network stack.
 class ChromeNetworkDelegate : public net::NetworkDelegate {
@@ -106,6 +110,10 @@ class ChromeNetworkDelegate : public net::NetworkDelegate {
     domain_reliability_monitor_ = domain_reliability_monitor;
   }
 
+  void set_prerender_tracker(prerender::PrerenderTracker* prerender_tracker) {
+    prerender_tracker_ = prerender_tracker;
+  }
+
   // Adds the Client Hints header to HTTP requests.
   void SetEnableClientHints();
 
@@ -228,6 +236,8 @@ class ChromeNetworkDelegate : public net::NetworkDelegate {
 
   bool first_request_;
 
+  prerender::PrerenderTracker* prerender_tracker_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeNetworkDelegate);
 };
 

chrome/browser/net/cookie_store_util.cc

@@ -12,6 +12,8 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/net/chrome_cookie_notification_details.h"
 #include "chrome/browser/net/evicted_domain_cookie_counter.h"
+#include "chrome/browser/prerender/prerender_manager.h"
+#include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/chrome_constants.h"
@@ -49,6 +51,13 @@ class ChromeCookieMonsterDelegate : public net::CookieMonsterDelegate {
                    this, cookie, removed, cause));
   }
 
+  virtual void OnLoaded() OVERRIDE {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&ChromeCookieMonsterDelegate::OnLoadedAsyncHelper,
+                   this));
+  }
+
  private:
   virtual ~ChromeCookieMonsterDelegate() {}
 
@@ -73,6 +82,16 @@ class ChromeCookieMonsterDelegate : public net::CookieMonsterDelegate {
     }
   }
 
+  void OnLoadedAsyncHelper() {
+    Profile* profile = profile_getter_.Run();
+    if (profile) {
+      prerender::PrerenderManager* prerender_manager =
+          prerender::PrerenderManagerFactory::GetForProfile(profile);
+      if (prerender_manager)
+        prerender_manager->OnCookieStoreLoaded();
+    }
+  }
+
   const base::Callback<Profile*(void)> profile_getter_;
 };
 

chrome/browser/net/evicted_domain_cookie_counter.cc

@@ -111,6 +111,11 @@ void EvictedDomainCookieCounter::OnCookieChanged(
     next_cookie_monster_delegate_->OnCookieChanged(cookie, removed, cause);
 }
 
+void EvictedDomainCookieCounter::OnLoaded() {
+  if (next_cookie_monster_delegate_.get())
+    next_cookie_monster_delegate_->OnLoaded();
+}
+
 // static
 EvictedDomainCookieCounter::EvictedCookieKey
     EvictedDomainCookieCounter::GetKey(const net::CanonicalCookie& cookie) {

chrome/browser/net/evicted_domain_cookie_counter.h

@@ -90,6 +90,7 @@ class EvictedDomainCookieCounter : public net::CookieMonster::Delegate {
   virtual void OnCookieChanged(const net::CanonicalCookie& cookie,
                                bool removed,
                                ChangeCause cause) OVERRIDE;
+  virtual void OnLoaded() OVERRIDE;
 
  private:
   // Identifier of an evicted cookie.

chrome/browser/net/evicted_domain_cookie_counter_unittest.cc

@@ -193,6 +193,8 @@ TEST_F(EvictedDomainCookieCounterTest, TestChain) {
       ++(*result_);
     }
 
+    virtual void OnLoaded() OVERRIDE {}
+
    private:
     virtual ~ChangedDelegateDummy() {}
 

chrome/browser/prerender/external_prerender_handler_android.cc

@@ -89,6 +89,17 @@ static jboolean HasPrerenderedUrl(JNIEnv* env,
   return prerender_manager->HasPrerenderedUrl(url, web_contents);
 }
 
+static jboolean HasCookieStoreLoaded(JNIEnv* env,
+                                     jclass clazz,
+                                     jobject jprofile) {
+  Profile* profile = ProfileAndroid::FromProfileAndroid(jprofile);
+  prerender::PrerenderManager* prerender_manager =
+      prerender::PrerenderManagerFactory::GetForProfile(profile);
+  if (!prerender_manager)
+    return false;
+  return prerender_manager->cookie_store_loaded();
+}
+
 ExternalPrerenderHandlerAndroid::ExternalPrerenderHandlerAndroid() {}
 
 ExternalPrerenderHandlerAndroid::~ExternalPrerenderHandlerAndroid() {}

chrome/browser/prerender/external_prerender_handler_android.h

@@ -45,6 +45,9 @@ class ExternalPrerenderHandlerAndroid {
                                 GURL url,
                                 content::WebContents* web_contents);
 
+  // Whether the cookie store associated with this profile has been loaded.
+  static bool HasCookieStoreLoaded(Profile* profile);
+
   static bool RegisterExternalPrerenderHandlerAndroid(JNIEnv* env);
 
  private: