In IE8 new windows opened within ChromeFrame via window.open calls at times bypass the host network stack.

In this case we don't get control over the navigation as window.open calls expect to carry the opener
relationship over to the new window. This basically means that navigation occurs in Chrome and the 
new tab/window created by the host attaches to the newly created ExternalTabContainer instance.

In IE8 the new tab opens in a new IE process, which basically uses a new automation channel to talk to
Chrome. The host network stack implementation routes network requests issued by registered render views
to the host browser over the automation channel. There is a timing window between the new window getting
created and issuing network requests and the channel being established with the new iexplore instance.
As a result network requests issued by the new window don't use the host network stack.

Fix is to register the render view and process as a pending view when we get notified about the new TabContents
in the original ExternalTabContainers implementation of TabContentsDelegate::AddNewContents. Any network requests
issued for this view would result in the corresponding URLRequestAutomationJob instances getting created as 
pending as well. When the host browser connects to the new ExternalTabContainer instance, we pass over the new
automation channel and tab handle to the URLRequestAutomationJob instances and resume them.

This fixes bug http://code.google.com/p/chromium/issues/detail?id=33516

Bug=33516
Test=Login to gmail in IE8 in ChromeFrame. Open up a new conversation and click on New Window which opens up
     a tear off window. This should not bring up the login prompt again.

Will think about a good approach to write a unit test for this behavior and send out a separate CL for that.

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

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

4 files changed, 202 insertions, 25 deletions

diff --git a/chrome/browser/automation/automation_resource_message_filter.cc b/chrome/browser/automation/automation_resource_message_filter.cc
index 6709ba2..77c47a9 100644
--- a/chrome/browser/automation/automation_resource_message_filter.cc
+++ b/chrome/browser/automation/automation_resource_message_filter.cc
@@ -6,6 +6,7 @@
 
 #include "base/histogram.h"
 #include "base/path_service.h"
+#include "base/stl_util-inl.h"
 #include "chrome/browser/automation/url_request_automation_job.h"
 #include "chrome/browser/net/url_request_failed_dns_job.h"
 #include "chrome/browser/net/url_request_mock_http_job.h"
@@ -111,23 +112,43 @@ bool AutomationResourceMessageFilter::RegisterRequest(
     NOTREACHED();
     return false;
   }
-
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
-  DCHECK(request_map_.end() == request_map_.find(job->id()));
-  request_map_[job->id()] = job;
+
+  // Register pending jobs in the pending request map for servicing later.
+  if (job->is_pending()) {
+    DCHECK(!ContainsKey(pending_request_map_, job->id()));
+    DCHECK(!ContainsKey(request_map_, job->id()));
+    pending_request_map_[job->id()] = job;
+  } else {
+    DCHECK(!ContainsKey(request_map_, job->id()));
+    DCHECK(!ContainsKey(pending_request_map_, job->id()));
+    request_map_[job->id()] = job;
+  }
+
   return true;
 }
 
 void AutomationResourceMessageFilter::UnRegisterRequest(
     URLRequestAutomationJob* job) {
+  if (!job) {
+    NOTREACHED();
+    return;
+  }
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
-  DCHECK(request_map_.find(job->id()) != request_map_.end());
-  request_map_.erase(job->id());
+
+  if (job->is_pending()) {
+    DCHECK(ContainsKey(pending_request_map_, job->id()));
+    pending_request_map_.erase(job->id());
+  } else {
+    DCHECK(ContainsKey(request_map_, job->id()));
+    request_map_.erase(job->id());
+  }
 }
 
 bool AutomationResourceMessageFilter::RegisterRenderView(
     int renderer_pid, int renderer_id, int tab_handle,
-    AutomationResourceMessageFilter* filter) {
+    AutomationResourceMessageFilter* filter,
+    bool pending_view) {
   if (!renderer_pid || !renderer_id || !tab_handle) {
     NOTREACHED();
     return false;
@@ -137,7 +158,7 @@ bool AutomationResourceMessageFilter::RegisterRenderView(
       ChromeThread::IO, FROM_HERE,
       NewRunnableFunction(
           AutomationResourceMessageFilter::RegisterRenderViewInIOThread,
-          renderer_pid, renderer_id, tab_handle, filter));
+          renderer_pid, renderer_id, tab_handle, filter, pending_view));
   return true;
 }
 
@@ -150,9 +171,26 @@ void AutomationResourceMessageFilter::UnRegisterRenderView(
           renderer_pid, renderer_id));
 }
 
+bool AutomationResourceMessageFilter::ResumePendingRenderView(
+    int renderer_pid, int renderer_id, int tab_handle,
+    AutomationResourceMessageFilter* filter) {
+  if (!renderer_pid || !renderer_id || !tab_handle) {
+    NOTREACHED();
+    return false;
+  }
+
+  ChromeThread::PostTask(
+      ChromeThread::IO, FROM_HERE,
+      NewRunnableFunction(
+          AutomationResourceMessageFilter::ResumePendingRenderViewInIOThread,
+          renderer_pid, renderer_id, tab_handle, filter));
+  return true;
+}
+
 void AutomationResourceMessageFilter::RegisterRenderViewInIOThread(
     int renderer_pid, int renderer_id,
-    int tab_handle, AutomationResourceMessageFilter* filter) {
+    int tab_handle, AutomationResourceMessageFilter* filter,
+    bool pending_view) {
   RenderViewMap::iterator automation_details_iter(
       filtered_render_views_.find(RendererId(renderer_pid, renderer_id)));
   if (automation_details_iter != filtered_render_views_.end()) {
@@ -160,10 +198,11 @@ void AutomationResourceMessageFilter::RegisterRenderViewInIOThread(
     automation_details_iter->second.ref_count++;
   } else {
     filtered_render_views_[RendererId(renderer_pid, renderer_id)] =
-        AutomationDetails(tab_handle, filter);
+        AutomationDetails(tab_handle, filter, pending_view);
   }
 }
 
+// static
 void AutomationResourceMessageFilter::UnRegisterRenderViewInIOThread(
     int renderer_pid, int renderer_id) {
   RenderViewMap::iterator automation_details_iter(
@@ -181,6 +220,36 @@ void AutomationResourceMessageFilter::UnRegisterRenderViewInIOThread(
   }
 }
 
+// static
+bool AutomationResourceMessageFilter::ResumePendingRenderViewInIOThread(
+    int renderer_pid, int renderer_id, int tab_handle,
+    AutomationResourceMessageFilter* filter) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
+
+  RenderViewMap::iterator automation_details_iter(
+      filtered_render_views_.find(RendererId(renderer_pid, renderer_id)));
+
+  if (automation_details_iter == filtered_render_views_.end()) {
+    NOTREACHED() << "Failed to find pending view for renderer pid:"
+                 << renderer_pid
+                 << ", render view id:"
+                 << renderer_id;
+    return false;
+  }
+
+  DCHECK(automation_details_iter->second.is_pending_render_view);
+
+  AutomationResourceMessageFilter* old_filter =
+      automation_details_iter->second.filter;
+  DCHECK(old_filter != NULL);
+
+  filtered_render_views_[RendererId(renderer_pid, renderer_id)] =
+      AutomationDetails(tab_handle, filter, false);
+
+  ResumeJobsForPendingView(tab_handle, old_filter, filter);
+  return true;
+}
+
 bool AutomationResourceMessageFilter::LookupRegisteredRenderView(
     int renderer_pid, int renderer_id, AutomationDetails* details) {
   bool found = false;
@@ -242,3 +311,24 @@ void AutomationResourceMessageFilter::OnRecordHistograms(
   }
 }
 
+// static
+void AutomationResourceMessageFilter::ResumeJobsForPendingView(
+    int tab_handle,
+    AutomationResourceMessageFilter* old_filter,
+    AutomationResourceMessageFilter* new_filter) {
+  DCHECK(old_filter != NULL);
+  DCHECK(new_filter != NULL);
+
+  RequestMap pending_requests = old_filter->pending_request_map_;
+
+  for (RequestMap::iterator index = old_filter->pending_request_map_.begin();
+          index != old_filter->pending_request_map_.end(); index++) {
+    scoped_refptr<URLRequestAutomationJob> job = (*index).second;
+    DCHECK_EQ(job->message_filter(), old_filter);
+    DCHECK(job->is_pending());
+    // StartPendingJob will register the job with the new filter.
+    job->StartPendingJob(tab_handle, new_filter);
+  }
+
+  old_filter->pending_request_map_.clear();
+}
diff --git a/chrome/browser/automation/automation_resource_message_filter.h b/chrome/browser/automation/automation_resource_message_filter.h
index 4625534..9d0e8018 100644
--- a/chrome/browser/automation/automation_resource_message_filter.h
+++ b/chrome/browser/automation/automation_resource_message_filter.h
@@ -25,14 +25,20 @@ class AutomationResourceMessageFilter
  public:
   // Information needed to send IPCs through automation.
   struct AutomationDetails {
-    AutomationDetails() : tab_handle(0), ref_count(1) {}
-    AutomationDetails(int tab, AutomationResourceMessageFilter* flt)
-      : tab_handle(tab), ref_count(1), filter(flt) {
+    AutomationDetails() : tab_handle(0), ref_count(1),
+                          is_pending_render_view(false) {}
+    AutomationDetails(int tab, AutomationResourceMessageFilter* flt,
+                      bool pending_view)
+      : tab_handle(tab), ref_count(1), filter(flt),
+        is_pending_render_view(pending_view) {
     }
 
     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;
   };
 
   // Create the filter.
@@ -61,10 +67,20 @@ class AutomationResourceMessageFilter
   virtual void UnRegisterRequest(URLRequestAutomationJob* job);
 
   // Can be called from the UI thread.
+  // The pending_view parameter should be true if network requests initiated by
+  // this render view need to be paused waiting for an acknowledgement from
+  // the external host.
   static bool RegisterRenderView(int renderer_pid, int renderer_id,
-      int tab_handle, AutomationResourceMessageFilter* filter);
+      int tab_handle, AutomationResourceMessageFilter* filter,
+      bool pending_view);
   static void UnRegisterRenderView(int renderer_pid, int renderer_id);
 
+  // Can be called from the UI thread.
+  // Resumes pending render views, i.e. network requests issued by this view
+  // can now be serviced.
+  static bool ResumePendingRenderView(int renderer_pid, int renderer_id,
+      int tab_handle, AutomationResourceMessageFilter* filter);
+
   // Called only on the IO thread.
   static bool LookupRegisteredRenderView(
       int renderer_pid, int renderer_id, AutomationDetails* details);
@@ -80,14 +96,26 @@ class AutomationResourceMessageFilter
   bool GetAutomationRequestId(int request_id, int* automation_request_id);
 
   static void RegisterRenderViewInIOThread(int renderer_pid, int renderer_id,
-      int tab_handle, AutomationResourceMessageFilter* filter);
+      int tab_handle, AutomationResourceMessageFilter* filter,
+      bool pending_view);
   static void UnRegisterRenderViewInIOThread(int renderer_pid, int renderer_id);
 
+  static bool ResumePendingRenderViewInIOThread(
+      int renderer_pid, int renderer_id, int tab_handle,
+      AutomationResourceMessageFilter* filter);
+
  private:
   void OnSetFilteredInet(bool enable);
   void OnGetFilteredInetHitCount(int* hit_count);
   void OnRecordHistograms(const std::vector<std::string>& histogram_list);
 
+  // Resumes pending jobs from the old AutomationResourceMessageFilter instance
+  // passed in.
+  static void ResumeJobsForPendingView(
+      int tab_handle,
+      AutomationResourceMessageFilter* old_filter,
+      AutomationResourceMessageFilter* new_filter);
+
   // A unique renderer id is a combination of renderer process id and
   // it's routing id.
   struct RendererId {
@@ -115,6 +143,10 @@ class AutomationResourceMessageFilter
   // Map of outstanding requests.
   RequestMap request_map_;
 
+  // Map of pending requests, i.e. requests which were waiting for the external
+  // host to connect back.
+  RequestMap pending_request_map_;
+
   // Map of render views interested in diverting url requests over automation.
   static RenderViewMap filtered_render_views_;
 
diff --git a/chrome/browser/automation/url_request_automation_job.cc b/chrome/browser/automation/url_request_automation_job.cc
index f0ec8eb..a22d05a 100644
--- a/chrome/browser/automation/url_request_automation_job.cc
+++ b/chrome/browser/automation/url_request_automation_job.cc
@@ -68,13 +68,14 @@ bool IsParsedCookiePresentInCookieHeader(
 }  // end namespace
 
 URLRequestAutomationJob::URLRequestAutomationJob(URLRequest* request, int tab,
-    int request_id, AutomationResourceMessageFilter* filter)
+    int request_id, AutomationResourceMessageFilter* filter, bool is_pending)
     : URLRequestJob(request),
       tab_(tab),
       message_filter_(filter),
       pending_buf_size_(0),
       redirect_status_(0),
-      request_id_(request_id) {
+      request_id_(request_id),
+      is_pending_(is_pending) {
   DLOG(INFO) << "URLRequestAutomationJob create. Count: " << ++instance_count_;
   DCHECK(message_filter_ != NULL);
 
@@ -127,7 +128,8 @@ URLRequestJob* URLRequestAutomationJob::Factory(URLRequest* request,
       if (AutomationResourceMessageFilter::LookupRegisteredRenderView(
               child_id, route_id, &details)) {
         URLRequestAutomationJob* job = new URLRequestAutomationJob(request,
-            details.tab_handle, request_info->request_id(), details.filter);
+            details.tab_handle, request_info->request_id(), details.filter,
+            details.is_pending_render_view);
         return job;
       }
     }
@@ -142,16 +144,30 @@ URLRequestJob* URLRequestAutomationJob::Factory(URLRequest* request,
 
 // URLRequestJob Implementation.
 void URLRequestAutomationJob::Start() {
-  // Start reading asynchronously so that all error reporting and data
-  // callbacks happen as they would for network requests.
-  MessageLoop::current()->PostTask(FROM_HERE, NewRunnableMethod(
-      this, &URLRequestAutomationJob::StartAsync));
+  if (!is_pending()) {
+    // Start reading asynchronously so that all error reporting and data
+    // callbacks happen as they would for network requests.
+    MessageLoop::current()->PostTask(FROM_HERE, NewRunnableMethod(
+        this, &URLRequestAutomationJob::StartAsync));
+  } else {
+    // If this is a pending job, then register it immediately with the message
+    // filter so it can be serviced later when we receive a request from the
+    // external host to connect to the corresponding external tab.
+    message_filter_->RegisterRequest(this);
+  }
 }
 
 void URLRequestAutomationJob::Kill() {
-  if (message_filter_.get()) {
-    message_filter_->Send(new AutomationMsg_RequestEnd(0, tab_, id_,
-        URLRequestStatus(URLRequestStatus::CANCELED, net::ERR_ABORTED)));
+  if (!is_pending()) {
+    if (message_filter_.get()) {
+      message_filter_->Send(new AutomationMsg_RequestEnd(0, tab_, id_,
+          URLRequestStatus(URLRequestStatus::CANCELED, net::ERR_ABORTED)));
+    }
+  } else {
+    // If this is a pending job, then register it from the message filter to
+    // ensure that it is not serviced when the external host connects to the
+    // corresponding external tab.
+    message_filter_->UnRegisterRequest(this);
   }
   DisconnectFromMessageFilter();
   URLRequestJob::Kill();
@@ -161,6 +177,10 @@ bool URLRequestAutomationJob::ReadRawData(
     net::IOBuffer* buf, int buf_size, int* bytes_read) {
   DLOG(INFO) << "URLRequestAutomationJob: " <<
       request_->url().spec() << " - read pending: " << buf_size;
+
+  // We should not receive a read request for a pending job.
+  DCHECK(!is_pending());
+
   pending_buf_ = buf;
   pending_buf_size_ = buf_size;
 
@@ -428,6 +448,9 @@ void URLRequestAutomationJob::StartAsync() {
   if (is_done())
     return;
 
+  // We should not receive a Start request for a pending job.
+  DCHECK(!is_pending());
+
   if (!request_) {
     NotifyStartError(URLRequestStatus(URLRequestStatus::FAILED,
         net::ERR_FAILED));
@@ -486,6 +509,16 @@ void URLRequestAutomationJob::DisconnectFromMessageFilter() {
   }
 }
 
+void URLRequestAutomationJob::StartPendingJob(
+    int new_tab_handle,
+    AutomationResourceMessageFilter* new_filter) {
+  DCHECK(new_filter != NULL);
+  tab_ = new_tab_handle;
+  message_filter_ = new_filter;
+  is_pending_ = false;
+  Start();
+}
+
 // static
 bool URLRequestAutomationJob::IsCookiePresentInCookieHeader(
     const std::string& cookie_name,
diff --git a/chrome/browser/automation/url_request_automation_job.h b/chrome/browser/automation/url_request_automation_job.h
index 9d96a71..b44c001 100644
--- a/chrome/browser/automation/url_request_automation_job.h
+++ b/chrome/browser/automation/url_request_automation_job.h
@@ -7,6 +7,8 @@
 #define CHROME_BROWSER_AUTOMATION_URL_REQUEST_AUTOMATION_JOB_H_
 
 #include <vector>
+
+#include "chrome/browser/automation/automation_resource_message_filter.h"
 #include "chrome/common/ref_counted_util.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/url_request.h"
@@ -24,7 +26,8 @@ struct AutomationURLResponse;
 class URLRequestAutomationJob : public URLRequestJob {
  public:
   URLRequestAutomationJob(URLRequest* request, int tab, int request_id,
-                          AutomationResourceMessageFilter* filter);
+                          AutomationResourceMessageFilter* filter,
+                          bool is_pending);
 
   // Register our factory for HTTP/HTTPs requests.
   static bool EnsureProtocolFactoryRegistered();
@@ -62,6 +65,20 @@ class URLRequestAutomationJob : public URLRequestJob {
   // it appends "; path=/" to the cookie string.
   static void SetCookiePathToRootIfNotPresent(std::string* cookie_string);
 
+  bool is_pending() const {
+    return is_pending_;
+  }
+
+  AutomationResourceMessageFilter* message_filter() const {
+    return message_filter_;
+  }
+
+  // Resumes a job, which was waiting for the external host to connect to the
+  // automation channel. This is to ensure that this request gets routed to the
+  // external host.
+  void StartPendingJob(int new_tab_handle,
+                       AutomationResourceMessageFilter* new_filter);
+
  protected:
   // Protected URLRequestJob override.
   virtual bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read);
@@ -100,6 +117,11 @@ class URLRequestAutomationJob : public URLRequestJob {
   static URLRequest::ProtocolFactory* old_http_factory_;
   static URLRequest::ProtocolFactory* old_https_factory_;
 
+  // Set to true if the job is waiting for the external host to connect to the
+  // automation channel, which will be used for routing the network requests to
+  // the host.
+  bool is_pending_;
+
   DISALLOW_COPY_AND_ASSIGN(URLRequestAutomationJob);
 };