ChromeFrame currently overrides the request context for intercepting network requests and cookie requests to route them

over the automation channel. This adds needless complexity and race conditions between registering a request context for
a renderer process as the first one wins. 

We no longer override the request context in ChromeFrame. For cookie requests we look up the registered render view map
and on finding one we route the request over the automation channel.

Fixes bug http://code.google.com/p/chromium/issues/detail?id=51103

Bug=51103

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

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

5 files changed, 122 insertions, 276 deletions

diff --git a/chrome/browser/automation/automation_profile_impl.cc b/chrome/browser/automation/automation_profile_impl.cc
deleted file mode 100644
index 0b14c7e..0000000
--- a/chrome/browser/automation/automation_profile_impl.cc
+++ /dev/null
@@ -1,203 +0,0 @@
-// Copyright (c) 2006-2009 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/automation/automation_profile_impl.h"
-
-#include <map>
-
-#include "chrome/browser/automation/automation_resource_message_filter.h"
-#include "chrome/browser/chrome_thread.h"
-#include "chrome/browser/net/chrome_url_request_context.h"
-#include "chrome/browser/profile.h"
-#include "net/base/net_errors.h"
-#include "net/url_request/url_request_context.h"
-#include "chrome/test/automation/automation_messages.h"
-
-namespace AutomationRequestContext {
-
-// A special Request context for automation. Substitute a few things
-// like cookie store, proxy settings etc to handle them differently
-// for automation.
-class AutomationURLRequestContext : public ChromeURLRequestContext {
- public:
-  AutomationURLRequestContext(ChromeURLRequestContext* original_context,
-                              net::CookieStore* automation_cookie_store,
-                              net::CookiePolicy* automation_cookie_policy)
-      : ChromeURLRequestContext(original_context),
-        // We must hold a reference to |original_context|, since many
-        // of the dependencies that ChromeURLRequestContext(original_context)
-        // copied are scoped to |original_context|.
-        original_context_(original_context) {
-    cookie_policy_ = automation_cookie_policy;
-    cookie_store_ = automation_cookie_store;
-  }
-
-  virtual bool IsExternal() const {
-    return true;
-  }
-
- private:
-  virtual ~AutomationURLRequestContext() {
-    // Clear out members before calling base class dtor since we don't
-    // own any of them.
-
-    // Clear URLRequestContext members.
-    host_resolver_ = NULL;
-    proxy_service_ = NULL;
-    http_transaction_factory_ = NULL;
-    ftp_transaction_factory_ = NULL;
-    cookie_store_ = NULL;
-    transport_security_state_ = NULL;
-  }
-
-  scoped_refptr<ChromeURLRequestContext> original_context_;
-  DISALLOW_COPY_AND_ASSIGN(AutomationURLRequestContext);
-};
-
-// CookieStore specialization to have automation specific
-// behavior for cookies.
-class AutomationCookieStore : public net::CookieStore {
- public:
-  AutomationCookieStore(net::CookieStore* original_cookie_store,
-                        AutomationResourceMessageFilter* automation_client,
-                        int tab_handle)
-      : original_cookie_store_(original_cookie_store),
-        automation_client_(automation_client),
-        tab_handle_(tab_handle) {
-  }
-
-  virtual ~AutomationCookieStore() {
-    DLOG(INFO) << "In " << __FUNCTION__;
-  }
-
-  // CookieStore implementation.
-  virtual bool SetCookieWithOptions(const GURL& url,
-                                    const std::string& cookie_line,
-                                    const net::CookieOptions& options) {
-    // The cookie_string_ is available only once, i.e. once it is read by
-    // it is invalidated.
-    cookie_string_ = cookie_line;
-    return true;
-  }
-
-  virtual std::string GetCookiesWithOptions(const GURL& url,
-                                            const net::CookieOptions& options) {
-    return cookie_string_;
-  }
-
-  virtual void DeleteCookie(const GURL& url,
-                            const std::string& cookie_name) {
-    NOTREACHED() << "Should not get called for an automation profile";
-  }
-
-  virtual net::CookieMonster* GetCookieMonster() {
-    NOTREACHED() << "Should not get called for an automation profile";
-    return NULL;
-  }
-
- protected:
-  net::CookieStore* original_cookie_store_;
-  scoped_refptr<AutomationResourceMessageFilter> automation_client_;
-  int tab_handle_;
-  std::string cookie_string_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(AutomationCookieStore);
-};
-
-// CookiePolicy specialization for automation specific cookie policies.
-class AutomationCookiePolicy : public net::CookiePolicy {
- public:
-  AutomationCookiePolicy(AutomationResourceMessageFilter* automation_client,
-                         int tab_handle, net::CookieStore* cookie_store)
-      : automation_client_(automation_client),
-        tab_handle_(tab_handle),
-        cookie_store_(cookie_store) {}
-
-  virtual int CanGetCookies(const GURL& url,
-                            const GURL& first_party_for_cookies,
-                            net::CompletionCallback* callback) {
-    DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
-
-    AutomationResourceMessageFilter::GetCookiesForUrl(tab_handle_, url,
-                                                      callback,
-                                                      cookie_store_.get());
-    return net::ERR_IO_PENDING;
-  }
-
-  virtual int CanSetCookie(const GURL& url,
-                           const GURL& first_party_for_cookies,
-                           const std::string& cookie_line,
-                           net::CompletionCallback* callback) {
-    AutomationResourceMessageFilter::SetCookiesForUrl(tab_handle_, url,
-                                                      cookie_line,
-                                                      callback);
-    return net::ERR_IO_PENDING;
-  }
-
- private:
-  scoped_refptr<AutomationResourceMessageFilter> automation_client_;
-  int tab_handle_;
-  scoped_refptr<net::CookieStore> cookie_store_;
-
-  DISALLOW_COPY_AND_ASSIGN(AutomationCookiePolicy);
-};
-
-class Factory : public ChromeURLRequestContextFactory {
- public:
-  Factory(ChromeURLRequestContextGetter* original_context_getter,
-          Profile* profile,
-          AutomationResourceMessageFilter* automation_client,
-          int tab_handle)
-      : ChromeURLRequestContextFactory(profile),
-        original_context_getter_(original_context_getter),
-        automation_client_(automation_client),
-        tab_handle_(tab_handle) {
-  }
-
-  virtual ChromeURLRequestContext* Create() {
-    ChromeURLRequestContext* original_context =
-        original_context_getter_->GetIOContext();
-
-    // Create an automation cookie store.
-    scoped_refptr<net::CookieStore> automation_cookie_store =
-        new AutomationCookieStore(original_context->cookie_store(),
-                                  automation_client_,
-                                  tab_handle_);
-
-    // Create an automation cookie policy.
-    AutomationCookiePolicy* automation_cookie_policy =
-        new AutomationCookiePolicy(automation_client_,
-                                   tab_handle_,
-                                   automation_cookie_store);
-
-    return new AutomationURLRequestContext(original_context,
-                                           automation_cookie_store,
-                                           automation_cookie_policy);
-  }
-
- private:
-  scoped_refptr<ChromeURLRequestContextGetter> original_context_getter_;
-  scoped_refptr<AutomationResourceMessageFilter> automation_client_;
-  int tab_handle_;
-};
-
-ChromeURLRequestContextGetter* CreateAutomationURLRequestContextForTab(
-    int tab_handle,
-    Profile* profile,
-    AutomationResourceMessageFilter* automation_client) {
-  ChromeURLRequestContextGetter* original_context =
-      static_cast<ChromeURLRequestContextGetter*>(
-          profile->GetRequestContext());
-
-  ChromeURLRequestContextGetter* request_context =
-      new ChromeURLRequestContextGetter(
-          NULL,  // Don't register an observer on PrefService.
-          new Factory(original_context, profile, automation_client,
-                      tab_handle));
-  return request_context;
-}
-
-}  // namespace AutomationRequestContext
-
diff --git a/chrome/browser/automation/automation_profile_impl.h b/chrome/browser/automation/automation_profile_impl.h
deleted file mode 100644
index a3ff9f0..0000000
--- a/chrome/browser/automation/automation_profile_impl.h
+++ /dev/null
@@ -1,26 +0,0 @@
-// Copyright (c) 2009 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.
-
-#ifndef CHROME_BROWSER_AUTOMATION_AUTOMATION_PROFILE_IMPL_H_
-#define CHROME_BROWSER_AUTOMATION_AUTOMATION_PROFILE_IMPL_H_
-#pragma once
-
-#include "ipc/ipc_message.h"
-
-class Profile;
-class ChromeURLRequestContextGetter;
-class AutomationResourceMessageFilter;
-
-namespace AutomationRequestContext {
-
-// Returns the URL request context to be used by HTTP requests handled over
-// the automation channel.
-ChromeURLRequestContextGetter* CreateAutomationURLRequestContextForTab(
-    int tab_handle,
-    Profile* profile,
-    AutomationResourceMessageFilter* automation_client);
-
-}
-
-#endif  // CHROME_BROWSER_AUTOMATION_AUTOMATION_PROFILE_IMPL_H_
diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index bfe879f..12371bc6 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -1251,12 +1251,7 @@ void AutomationProvider::GetCookies(const GURL& url, int handle,
     NavigationController* tab = tab_tracker_->GetResource(handle);
 
     // Since we are running on the UI thread don't call GetURLRequestContext().
-    scoped_refptr<URLRequestContextGetter> request_context =
-        tab->tab_contents()->request_context();
-    if (!request_context.get())
-      request_context = tab->profile()->GetRequestContext();
-
-    *value = GetCookiesForURL(url, request_context.get());
+    *value = GetCookiesForURL(url, tab->profile()->GetRequestContext());
     *value_size = static_cast<int>(value->size());
   }
 }
@@ -1270,12 +1265,7 @@ void AutomationProvider::SetCookie(const GURL& url,
   if (url.is_valid() && tab_tracker_->ContainsHandle(handle)) {
     NavigationController* tab = tab_tracker_->GetResource(handle);
 
-    scoped_refptr<URLRequestContextGetter> request_context =
-        tab->tab_contents()->request_context();
-    if (!request_context.get())
-      request_context = tab->profile()->GetRequestContext();
-
-    if (SetCookieForURL(url, value, request_context.get()))
+    if (SetCookieForURL(url, value, tab->profile()->GetRequestContext()))
       *response_value = 1;
   }
 }
diff --git a/chrome/browser/automation/automation_resource_message_filter.cc b/chrome/browser/automation/automation_resource_message_filter.cc
index b570152..c9237c8 100644
--- a/chrome/browser/automation/automation_resource_message_filter.cc
+++ b/chrome/browser/automation/automation_resource_message_filter.cc
@@ -18,7 +18,6 @@
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/test/automation/automation_messages.h"
 #include "googleurl/src/gurl.h"
-#include "net/base/cookie_store.h"
 #include "net/base/net_errors.h"
 #include "net/url_request/url_request_filter.h"
 
@@ -33,6 +32,57 @@ base::LazyInstance<AutomationResourceMessageFilter::CompletionCallbackMap>
 int AutomationResourceMessageFilter::unique_request_id_ = 1;
 int AutomationResourceMessageFilter::next_completion_callback_id_ = 0;
 
+// CookieStore specialization to enable fetching and setting cookies over the
+// automation channel. This cookie store is transient i.e. it maintains cookies
+// for the duration of the current request to set or get cookies from the
+// renderer.
+class AutomationCookieStore : public net::CookieStore {
+ public:
+  AutomationCookieStore(AutomationResourceMessageFilter* automation_client,
+                        int tab_handle)
+      : automation_client_(automation_client),
+        tab_handle_(tab_handle) {
+  }
+
+  virtual ~AutomationCookieStore() {
+    DLOG(INFO) << "In " << __FUNCTION__;
+  }
+
+  // CookieStore implementation.
+  virtual bool SetCookieWithOptions(const GURL& url,
+                                    const std::string& cookie_line,
+                                    const net::CookieOptions& options) {
+    // The cookie_string_ is available only once, i.e. once it is read by
+    // it is invalidated.
+    cookie_string_ = cookie_line;
+    return true;
+  }
+
+  virtual std::string GetCookiesWithOptions(const GURL& url,
+                                            const net::CookieOptions& options) {
+    return cookie_string_;
+  }
+
+  virtual void DeleteCookie(const GURL& url,
+                            const std::string& cookie_name) {
+    NOTREACHED() << "Should not get called for an automation profile";
+  }
+
+  virtual net::CookieMonster* GetCookieMonster() {
+    NOTREACHED() << "Should not get called for an automation profile";
+    return NULL;
+  }
+
+ protected:
+  scoped_refptr<AutomationResourceMessageFilter> automation_client_;
+  int tab_handle_;
+  std::string cookie_string_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AutomationCookieStore);
+};
+
+
 AutomationResourceMessageFilter::AutomationResourceMessageFilter()
     : channel_(NULL) {
   // Ensure that an instance of the callback map is created.
@@ -209,16 +259,22 @@ void AutomationResourceMessageFilter::RegisterRenderViewInIOThread(
     int renderer_pid, int renderer_id,
     int tab_handle, AutomationResourceMessageFilter* filter,
     bool pending_view) {
+  RendererId renderer_key(renderer_pid, renderer_id);
+
+  scoped_refptr<net::CookieStore> cookie_store =
+      new AutomationCookieStore(filter, tab_handle);
+
   RenderViewMap::iterator automation_details_iter(
-      filtered_render_views_.Get().find(RendererId(renderer_pid,
-                                                   renderer_id)));
+      filtered_render_views_.Get().find(renderer_key));
   if (automation_details_iter != filtered_render_views_.Get().end()) {
     DCHECK(automation_details_iter->second.ref_count > 0);
     automation_details_iter->second.ref_count++;
   } else {
-    filtered_render_views_.Get()[RendererId(renderer_pid, renderer_id)] =
+    filtered_render_views_.Get()[renderer_key] =
         AutomationDetails(tab_handle, filter, pending_view);
   }
+
+  filtered_render_views_.Get()[renderer_key].set_cookie_store(cookie_store);
 }
 
 // static
@@ -246,9 +302,10 @@ bool AutomationResourceMessageFilter::ResumePendingRenderViewInIOThread(
     AutomationResourceMessageFilter* filter) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
 
+  RendererId renderer_key(renderer_pid, renderer_id);
+
   RenderViewMap::iterator automation_details_iter(
-      filtered_render_views_.Get().find(RendererId(renderer_pid,
-                                                   renderer_id)));
+      filtered_render_views_.Get().find(renderer_key));
 
   if (automation_details_iter == filtered_render_views_.Get().end()) {
     NOTREACHED() << "Failed to find pending view for renderer pid:"
@@ -260,13 +317,18 @@ bool AutomationResourceMessageFilter::ResumePendingRenderViewInIOThread(
 
   DCHECK(automation_details_iter->second.is_pending_render_view);
 
+  scoped_refptr<net::CookieStore> cookie_store =
+      new AutomationCookieStore(filter, tab_handle);
+
   AutomationResourceMessageFilter* old_filter =
       automation_details_iter->second.filter;
   DCHECK(old_filter != NULL);
 
-  filtered_render_views_.Get()[RendererId(renderer_pid, renderer_id)] =
+  filtered_render_views_.Get()[renderer_key] =
       AutomationDetails(tab_handle, filter, false);
 
+  filtered_render_views_.Get()[renderer_key].set_cookie_store(cookie_store);
+
   ResumeJobsForPendingView(tab_handle, old_filter, filter);
   return true;
 }
@@ -332,42 +394,42 @@ void AutomationResourceMessageFilter::OnRecordHistograms(
   }
 }
 
-void AutomationResourceMessageFilter::GetCookiesForUrl(
-    int tab_handle, const GURL& url, net::CompletionCallback* callback,
-    net::CookieStore* cookie_store) {
-  DCHECK(callback != NULL);
-  DCHECK(cookie_store != NULL);
-
+bool AutomationResourceMessageFilter::GetCookiesForUrl(
+    const GURL& url, net::CompletionCallback* callback) {
   GetCookiesCompletion* get_cookies_callback =
       static_cast<GetCookiesCompletion*>(callback);
 
+  RendererId renderer_key(get_cookies_callback->render_process_id(),
+      get_cookies_callback->render_view_id());
+
   RenderViewMap::iterator automation_details_iter(
-        filtered_render_views_.Get().find(RendererId(
-            get_cookies_callback->render_process_id(),
-            get_cookies_callback->render_view_id())));
+        filtered_render_views_.Get().find(renderer_key));
+
   if (automation_details_iter == filtered_render_views_.Get().end()) {
-    OnGetCookiesHostResponseInternal(tab_handle, false, url, "", callback,
-                                     cookie_store);
-    return;
+    return false;
   }
 
   DCHECK(automation_details_iter->second.filter != NULL);
+  DCHECK(automation_details_iter->second.cookie_store_.get() != NULL);
 
   int completion_callback_id = GetNextCompletionCallbackId();
   DCHECK(!ContainsKey(completion_callback_map_.Get(), completion_callback_id));
 
   CookieCompletionInfo cookie_info;
   cookie_info.completion_callback = callback;
-  cookie_info.cookie_store = cookie_store;
+  cookie_info.cookie_store = automation_details_iter->second.cookie_store_;
 
   completion_callback_map_.Get()[completion_callback_id] = cookie_info;
 
+  DCHECK(automation_details_iter->second.filter != NULL);
+
   if (automation_details_iter->second.filter) {
     automation_details_iter->second.filter->Send(
         new AutomationMsg_GetCookiesFromHost(
             0, automation_details_iter->second.tab_handle, url,
             completion_callback_id));
   }
+  return true;
 }
 
 void AutomationResourceMessageFilter::OnGetCookiesHostResponse(
@@ -377,6 +439,7 @@ void AutomationResourceMessageFilter::OnGetCookiesHostResponse(
       completion_callback_map_.Get().find(cookie_id);
   if (index != completion_callback_map_.Get().end()) {
     net::CompletionCallback* callback = index->second.completion_callback;
+
     scoped_refptr<net::CookieStore> cookie_store = index->second.cookie_store;
 
     DCHECK(callback != NULL);
@@ -398,6 +461,11 @@ void AutomationResourceMessageFilter::OnGetCookiesHostResponseInternal(
   DCHECK(callback);
   DCHECK(cookie_store);
 
+  GetCookiesCompletion* get_cookies_callback =
+      static_cast<GetCookiesCompletion*>(callback);
+
+  get_cookies_callback->set_cookie_store(cookie_store);
+
   // Set the cookie in the cookie store so that the callback can read it.
   cookie_store->SetCookieWithOptions(url, cookies, net::CookieOptions());
 
@@ -409,8 +477,8 @@ void AutomationResourceMessageFilter::OnGetCookiesHostResponseInternal(
   cookie_store->SetCookieWithOptions(url, "", net::CookieOptions());
 }
 
-void AutomationResourceMessageFilter::SetCookiesForUrl(
-    int tab_handle, const GURL&url, const std::string& cookie_line,
+bool AutomationResourceMessageFilter::SetCookiesForUrl(
+    const GURL& url, const std::string& cookie_line,
     net::CompletionCallback* callback) {
   SetCookieCompletion* set_cookies_callback =
       static_cast<SetCookieCompletion*>(callback);
@@ -420,14 +488,19 @@ void AutomationResourceMessageFilter::SetCookiesForUrl(
             set_cookies_callback->render_process_id(),
             set_cookies_callback->render_view_id())));
 
-  if (automation_details_iter != filtered_render_views_.Get().end()) {
-    DCHECK(automation_details_iter->second.filter != NULL);
+  if (automation_details_iter == filtered_render_views_.Get().end()) {
+    return false;
+  }
 
-    if (automation_details_iter->second.filter) {
-      automation_details_iter->second.filter->Send(
-          new AutomationMsg_SetCookieAsync(0, tab_handle, url, cookie_line));
-    }
+  DCHECK(automation_details_iter->second.filter != NULL);
+
+  if (automation_details_iter->second.filter) {
+    automation_details_iter->second.filter->Send(
+        new AutomationMsg_SetCookieAsync(
+            0, automation_details_iter->second.tab_handle, url, cookie_line));
   }
+
+  return true;
 }
 
 // static
diff --git a/chrome/browser/automation/automation_resource_message_filter.h b/chrome/browser/automation/automation_resource_message_filter.h
index 5ed2a81..c245f280 100644
--- a/chrome/browser/automation/automation_resource_message_filter.h
+++ b/chrome/browser/automation/automation_resource_message_filter.h
@@ -14,6 +14,7 @@
 #include "base/platform_thread.h"
 #include "ipc/ipc_channel_proxy.h"
 #include "net/base/completion_callback.h"
+#include "net/base/cookie_store.h"
 
 class URLRequestAutomationJob;
 class GURL;
@@ -41,12 +42,23 @@ class AutomationResourceMessageFilter
         is_pending_render_view(pending_view) {
     }
 
+    void set_cookie_store(net::CookieStore* cookie_store) {
+      cookie_store_ = cookie_store;
+    }
+
+    net::CookieStore* cookie_store() {
+      return cookie_store_.get();
+    }
+
     int tab_handle;
     int ref_count;
     scoped_refptr<AutomationResourceMessageFilter> filter;
     // Indicates whether network requests issued by this render view need to
     // be executed later.
     bool is_pending_render_view;
+
+    // The cookie store associated with this render view.
+    scoped_refptr<net::CookieStore> cookie_store_;
   };
 
   // Create the filter.
@@ -101,9 +113,13 @@ class AutomationResourceMessageFilter
 
   // Retrieves cookies for the url passed in from the external host. The
   // callback passed in is notified on success or failure asynchronously.
-  static void GetCookiesForUrl(int tab_handle, const GURL& url,
-                               net::CompletionCallback* callback,
-                               net::CookieStore* cookie_store);
+  // Returns true on success.
+  static bool GetCookiesForUrl(const GURL& url,
+                               net::CompletionCallback* callback);
+
+  // Sets cookies on the URL in the external host. Returns true on success.
+  static bool SetCookiesForUrl(const GURL& url, const std::string& cookie_line,
+                               net::CompletionCallback* callback);
 
   // This function gets invoked when we receive a response from the external
   // host for the cookie request sent in GetCookiesForUrl above. It sets the
@@ -113,10 +129,6 @@ class AutomationResourceMessageFilter
   void OnGetCookiesHostResponse(int tab_handle, bool success, const GURL& url,
                                 const std::string& cookies, int cookie_id);
 
-  // Set cookies in the external host.
-  static void SetCookiesForUrl(int tab_handle, const GURL&url,
-    const std::string& cookie_line, net::CompletionCallback* callback);
-
  protected:
   // Retrieves the automation request id for the passed in chrome request
   // id and returns it in the automation_request_id parameter.