ChromeFrame HTTP requests would randomly fail if we navigated to multiple HTTP sites. This was because

the automation resource message filter tracked HTTP requests based on the request ids which are generated
by the renderer process. As a result a new request would get created say with id 0, while an existing request
would end in ChromeFrame causing the new request to incorrectly shutdown.

Fix is to revert back to the original way of tracking requests with an auto incrementing id. The automation url
job maintains both ids now, i.e. the automation request id and the chrome request id. The download notification
receives the automation id and basically looks up the associated automation request id and sends the notification
back to ChromeFrame.

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

Other fixes in this CL include the following:-
1. The active document instance would never get destroyed. This was because we call ShowUI on the doc host 
   which maintains a reference. We need to call HideUI in Setsite of NULL, which releases the reference.

2. When the active x instance is shutting down we try to shutdown all running requests in the OnDestroy handler.
   To ensure that the request is deleted from the request map and released in the same thread which created it
   we post a task back to the ui thread which never runs as the window is being destroyed. Fix is to create
   a message only window with every urlmonrequest instance which supports task marshaling.

Tests in a future CL.

Bug=27401

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

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

5 files changed, 70 insertions, 8 deletions

diff --git a/chrome/browser/automation/automation_resource_message_filter.cc b/chrome/browser/automation/automation_resource_message_filter.cc
index e0512ca..6709ba2 100644
--- a/chrome/browser/automation/automation_resource_message_filter.cc
+++ b/chrome/browser/automation/automation_resource_message_filter.cc
@@ -21,6 +21,8 @@
 AutomationResourceMessageFilter::RenderViewMap
     AutomationResourceMessageFilter::filtered_render_views_;
 
+int AutomationResourceMessageFilter::unique_request_id_ = 1;
+
 AutomationResourceMessageFilter::AutomationResourceMessageFilter()
     : channel_(NULL) {
   ChromeThread::PostTask(
@@ -193,6 +195,37 @@ bool AutomationResourceMessageFilter::LookupRegisteredRenderView(
   return found;
 }
 
+bool AutomationResourceMessageFilter::GetAutomationRequestId(
+    int request_id, int* automation_request_id) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
+
+  RequestMap::iterator it = request_map_.begin();
+  while (it != request_map_.end()) {
+    URLRequestAutomationJob* job = it->second;
+    DCHECK(job);
+    if (job && job->request_id() == request_id) {
+      *automation_request_id = job->id();
+      return true;
+    }
+    it++;
+  }
+
+  return false;
+}
+
+bool AutomationResourceMessageFilter::SendDownloadRequestToHost(
+    int routing_id, int tab_handle, int request_id) {
+  int automation_request_id = 0;
+  bool valid_id = GetAutomationRequestId(request_id, &automation_request_id);
+  if (!valid_id) {
+    NOTREACHED() << "Invalid request id: " << request_id;
+    return false;
+  }
+
+  return Send(new AutomationMsg_DownloadRequestInHost(0, tab_handle,
+                                                      automation_request_id));
+}
+
 void AutomationResourceMessageFilter::OnSetFilteredInet(bool enable) {
   chrome_browser_net::SetUrlRequestMocksEnabled(enable);
 }
diff --git a/chrome/browser/automation/automation_resource_message_filter.h b/chrome/browser/automation/automation_resource_message_filter.h
index ceb6115..7bb03d6 100644
--- a/chrome/browser/automation/automation_resource_message_filter.h
+++ b/chrome/browser/automation/automation_resource_message_filter.h
@@ -39,6 +39,12 @@ class AutomationResourceMessageFilter
   AutomationResourceMessageFilter();
   virtual ~AutomationResourceMessageFilter();
 
+  // Returns a new automation request id. This is unique across all instances
+  // of AutomationResourceMessageFilter.
+  int NewAutomationRequestId() {
+    return base::subtle::Barrier_AtomicIncrement(&unique_request_id_, 1);
+  }
+
   // IPC::ChannelProxy::MessageFilter methods:
   virtual void OnFilterAdded(IPC::Channel* channel);
   virtual void OnChannelConnected(int32 peer_pid);
@@ -63,7 +69,16 @@ class AutomationResourceMessageFilter
   static bool LookupRegisteredRenderView(
       int renderer_pid, int renderer_id, AutomationDetails* details);
 
+  // Sends the download request to the automation host.
+  bool SendDownloadRequestToHost(int routing_id, int tab_handle,
+                                 int request_id);
+
  protected:
+  // Retrieves the automation request id for the passed in chrome request
+  // id and returns it in the automation_request_id parameter.
+  // Returns true on success.
+  bool GetAutomationRequestId(int request_id, int* automation_request_id);
+
   static void RegisterRenderViewInIOThread(int renderer_pid, int renderer_id,
       int tab_handle, AutomationResourceMessageFilter* filter);
   static void UnRegisterRenderViewInIOThread(int renderer_pid, int renderer_id);
@@ -94,6 +109,9 @@ class AutomationResourceMessageFilter
   // owned by this class.
   IPC::Channel* channel_;
 
+  // A unique request id per process.
+  static int unique_request_id_;
+
   // Map of outstanding requests.
   RequestMap request_map_;
 
diff --git a/chrome/browser/automation/url_request_automation_job.cc b/chrome/browser/automation/url_request_automation_job.cc
index fdd680f..f6d4ecd 100644
--- a/chrome/browser/automation/url_request_automation_job.cc
+++ b/chrome/browser/automation/url_request_automation_job.cc
@@ -52,13 +52,18 @@ URLRequest::ProtocolFactory* URLRequestAutomationJob::old_https_factory_
 URLRequestAutomationJob::URLRequestAutomationJob(URLRequest* request, int tab,
     int request_id, AutomationResourceMessageFilter* filter)
     : URLRequestJob(request),
-      id_(request_id),
       tab_(tab),
       message_filter_(filter),
       pending_buf_size_(0),
-      redirect_status_(0) {
+      redirect_status_(0),
+      request_id_(request_id) {
   DLOG(INFO) << "URLRequestAutomationJob create. Count: " << ++instance_count_;
-  DCHECK_NE(id_, -1);
+  DCHECK(message_filter_ != NULL);
+
+  if (message_filter_) {
+    id_ = message_filter_->NewAutomationRequestId();
+    DCHECK_NE(id_, 0);
+  }
 }
 
 URLRequestAutomationJob::~URLRequestAutomationJob() {
@@ -217,7 +222,8 @@ bool URLRequestAutomationJob::MayFilterMessage(const IPC::Message& message,
     case AutomationMsg_RequestEnd::ID: {
       void* iter = NULL;
       int tab = 0;
-      if (message.ReadInt(&iter, &tab) && message.ReadInt(&iter, request_id)) {
+      if (message.ReadInt(&iter, &tab) &&
+          message.ReadInt(&iter, request_id)) {
         return true;
       }
       break;
diff --git a/chrome/browser/automation/url_request_automation_job.h b/chrome/browser/automation/url_request_automation_job.h
index 0edd876..5865fb3 100644
--- a/chrome/browser/automation/url_request_automation_job.h
+++ b/chrome/browser/automation/url_request_automation_job.h
@@ -47,6 +47,10 @@ class URLRequestAutomationJob : public URLRequestJob {
     return id_;
   }
 
+  int request_id() const {
+    return request_id_;
+  }
+
  protected:
   // Protected URLRequestJob override.
   virtual bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read);
@@ -75,6 +79,7 @@ class URLRequestAutomationJob : public URLRequestJob {
   scoped_refptr<net::HttpResponseHeaders> headers_;
   std::string redirect_url_;
   int redirect_status_;
+  int request_id_;
 
   static int instance_count_;
 
diff --git a/chrome/browser/external_tab_container.cc b/chrome/browser/external_tab_container.cc
index 4440294..806c4c6 100644
--- a/chrome/browser/external_tab_container.cc
+++ b/chrome/browser/external_tab_container.cc
@@ -404,10 +404,10 @@ bool ExternalTabContainer::TakeFocus(bool reverse) {
 bool ExternalTabContainer::CanDownload(int request_id) {
   if (load_requests_via_automation_) {
     if (automation_) {
-      // NOTE: The request_id must be the same id as used by corresponding
-      // URLRequestAutomationJob instance to communicate with the host.
-      automation_->Send(new AutomationMsg_DownloadRequestInHost(0, tab_handle_,
-                                                                request_id));
+      ChromeThread::PostTask(ChromeThread::IO, FROM_HERE,
+          NewRunnableMethod(automation_resource_message_filter_,
+              &AutomationResourceMessageFilter::SendDownloadRequestToHost,
+              0, tab_handle_, request_id));
     }
   } else {
     DLOG(WARNING) << "Downloads are only supported with host browser network "