Links opened in ChromeFrame via the Open in New window operation would not render if the target

also was rendered in ChromeFrame. This was because we were not marshaling the IMoniker and IBindCtx
interfaces over to the ChromeFrame worker thread. The problem would only occur when we reused the
existing IMoniker.

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

We need to post the OnWorkerStop message after we finish stopping the urlmon request object to
prevent a race condition. We also need to pump messages in OnDestroy to ensure that CoUninitialize and
calls like IBinding::Abort which callback into this thread complete correctly.


Bug=30013

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

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

2 files changed, 237 insertions, 4 deletions

diff --git a/chrome_frame/chrome_frame_activex_base.h b/chrome_frame/chrome_frame_activex_base.h
index 5ba38f8..0ba55e5 100644
--- a/chrome_frame/chrome_frame_activex_base.h
+++ b/chrome_frame/chrome_frame_activex_base.h
@@ -133,6 +133,10 @@ class ATL_NO_VTABLE ProxyDIChromeFrameEvents
 
 extern bool g_first_launch_by_process_;
 
+// Posted when the worker thread used for handling URL requests in IE finishes
+// uninitialization.
+#define WM_WORKER_THREAD_UNINITIALIZED_MSG (WM_APP + 1)
+
 // Common implementation for ActiveX and Active Document
 template <class T, const CLSID& class_id>
 class ATL_NO_VTABLE ChromeFrameActivexBase :  // NOLINT
@@ -554,10 +558,23 @@ END_MSG_MAP()
   LRESULT OnDestroy(UINT message, WPARAM wparam, LPARAM lparam,
                     BOOL& handled) {  // NO_LINT
     if (worker_thread_.message_loop()) {
-      worker_thread_.message_loop()->PostTask(
-          FROM_HERE, NewRunnableMethod(this, &Base::OnWorkerStop));
       if (automation_client_.get())
         automation_client_->CleanupRequests();
+
+      worker_thread_.message_loop()->PostTask(
+          FROM_HERE, NewRunnableMethod(this, &Base::OnWorkerStop));
+
+      MSG msg = {0};
+      while (GetMessage(&msg, NULL, WM_USER,
+                        WM_WORKER_THREAD_UNINITIALIZED_MSG)) {
+        if (msg.hwnd == m_hWnd &&
+            msg.message == WM_WORKER_THREAD_UNINITIALIZED_MSG) {
+          break;
+        }
+
+        TranslateMessage(&msg);
+        DispatchMessage(&msg);
+      }
       worker_thread_.Stop();
     }
     return 0;
@@ -1090,6 +1107,7 @@ END_MSG_MAP()
 
   void OnWorkerStop() {
     CoUninitialize();
+    PostMessage(WM_WORKER_THREAD_UNINITIALIZED_MSG, 0, 0);
   }
 
   ScopedBstr url_;
diff --git a/chrome_frame/urlmon_url_request.cc b/chrome_frame/urlmon_url_request.cc
index 5970b63..b98ba53 100644
--- a/chrome_frame/urlmon_url_request.cc
+++ b/chrome_frame/urlmon_url_request.cc
@@ -5,6 +5,7 @@
 #include "chrome_frame/urlmon_url_request.h"
 
 #include <wininet.h>
+#include <urlmon.h>
 
 #include "base/scoped_ptr.h"
 #include "base/string_util.h"
@@ -22,6 +23,203 @@ static const LARGE_INTEGER kZero = {0};
 static const ULARGE_INTEGER kUnsignedZero = {0};
 int UrlmonUrlRequest::instance_count_ = 0;
 
+// This class wraps the IBindCtx interface which is passed in when our active
+// document object is instantiated. The IBindCtx interface is created on
+// the UI thread and hence cannot be used as is on the worker thread which
+// handles URL requests. We unmarshal the IBindCtx interface and invoke
+// the corresponding method on the unmarshaled object. The object implementing
+// the IBindCtx interface also implements IMarshal. However it seems to have a
+// bug where in subsequent download requests for the same URL fail. We work
+// around this issue by using the standard marshaler instead.
+class WrappedBindContext : public IBindCtx,
+                           public CComObjectRootEx<CComMultiThreadModel> {
+ public:
+  WrappedBindContext() {
+    DLOG(INFO) << "In " << __FUNCTION__;
+  }
+
+  ~WrappedBindContext() {
+    DLOG(INFO) << "In " << __FUNCTION__ << " : Destroying object: " << this;
+  }
+
+  BEGIN_COM_MAP(WrappedBindContext)
+    COM_INTERFACE_ENTRY(IBindCtx)
+    COM_INTERFACE_ENTRY_IID(IID_IAsyncBindCtx, WrappedBindContext)
+    COM_INTERFACE_ENTRY(IUnknown)
+  END_COM_MAP()
+
+  HRESULT Initialize(IBindCtx* context) {
+    DCHECK(context != NULL);
+    HRESULT hr = CoGetStandardMarshal(__uuidof(IBindCtx),
+                                      context,
+                                      MSHCTX_INPROC,
+                                      NULL,
+                                      MSHLFLAGS_NORMAL,
+                                      standard_marshal_.Receive());
+    if (FAILED(hr)) {
+      NOTREACHED() << __FUNCTION__
+                   << ": CoGetStandardMarshal failed. Error:"
+                   << hr;
+      return hr;
+    }
+
+    DCHECK(standard_marshal_.get() != NULL);
+    DCHECK(marshaled_stream_.get() == NULL);
+
+    CreateStreamOnHGlobal(NULL, TRUE, marshaled_stream_.Receive());
+    DCHECK(marshaled_stream_.get() != NULL);
+
+    hr = standard_marshal_->MarshalInterface(marshaled_stream_,
+                                             __uuidof(IBindCtx),
+                                             context,
+                                             MSHCTX_INPROC,
+                                             NULL,
+                                             MSHLFLAGS_NORMAL);
+    if (FAILED(hr)) {
+      NOTREACHED() << __FUNCTION__
+                   << ": MarshalInterface failed. Error:"
+                   << hr;
+    }
+    return hr;
+  }
+
+  STDMETHOD(RegisterObjectBound)(IUnknown* object) {
+    DLOG(INFO) << "In " << __FUNCTION__ << " for object: " << this;
+
+    ScopedComPtr<IBindCtx> bind_context;
+    HRESULT hr = GetMarshalledBindContext(bind_context.Receive());
+    if (bind_context.get()) {
+      hr = bind_context->RegisterObjectBound(object);
+    }
+    return hr;
+  }
+
+  STDMETHOD(RevokeObjectBound)(IUnknown* object) {
+    DLOG(INFO) << "In " << __FUNCTION__ << " for object: " << this;
+
+    ScopedComPtr<IBindCtx> bind_context;
+    HRESULT hr = GetMarshalledBindContext(bind_context.Receive());
+    if (bind_context.get()) {
+      hr = bind_context->RevokeObjectBound(object);
+    }
+    return hr;
+  }
+
+  STDMETHOD(ReleaseBoundObjects)() {
+    DLOG(INFO) << "In " << __FUNCTION__ << " for object: " << this;
+
+    ScopedComPtr<IBindCtx> bind_context;
+    HRESULT hr = GetMarshalledBindContext(bind_context.Receive());
+    if (bind_context.get()) {
+      hr = bind_context->ReleaseBoundObjects();
+    }
+    return hr;
+  }
+
+  STDMETHOD(SetBindOptions)(BIND_OPTS* bind_options) {
+    DLOG(INFO) << "In " << __FUNCTION__ << " for object: " << this;
+
+    ScopedComPtr<IBindCtx> bind_context;
+    HRESULT hr = GetMarshalledBindContext(bind_context.Receive());
+    if (bind_context.get()) {
+      hr = bind_context->SetBindOptions(bind_options);
+    }
+    return hr;
+  }
+
+  STDMETHOD(GetBindOptions)(BIND_OPTS* bind_options) {
+    DLOG(INFO) << "In " << __FUNCTION__ << " for object: " << this;
+
+    ScopedComPtr<IBindCtx> bind_context;
+    HRESULT hr = GetMarshalledBindContext(bind_context.Receive());
+    if (bind_context.get()) {
+      hr = bind_context->GetBindOptions(bind_options);
+    }
+    return hr;
+  }
+
+  STDMETHOD(GetRunningObjectTable)(IRunningObjectTable** table) {
+    DLOG(INFO) << "In " << __FUNCTION__ << " for object: " << this;
+
+    ScopedComPtr<IBindCtx> bind_context;
+    HRESULT hr = GetMarshalledBindContext(bind_context.Receive());
+    if (bind_context.get()) {
+      hr = bind_context->GetRunningObjectTable(table);
+    }
+    return hr;
+  }
+
+  STDMETHOD(RegisterObjectParam)(LPOLESTR key, IUnknown* object) {
+    DLOG(INFO) << "In " << __FUNCTION__ << " for object: " << this;
+
+    ScopedComPtr<IBindCtx> bind_context;
+    HRESULT hr = GetMarshalledBindContext(bind_context.Receive());
+    if (bind_context.get()) {
+      hr = bind_context->RegisterObjectParam(key, object);
+    }
+    return hr;
+  }
+
+  STDMETHOD(GetObjectParam)(LPOLESTR key, IUnknown** object) {
+    DLOG(INFO) << "In " << __FUNCTION__ << " for object: " << this;
+
+    ScopedComPtr<IBindCtx> bind_context;
+    HRESULT hr = GetMarshalledBindContext(bind_context.Receive());
+    if (bind_context.get()) {
+      hr = bind_context->GetObjectParam(key, object);
+    }
+    return hr;
+  }
+
+  STDMETHOD(EnumObjectParam)(IEnumString** enum_string) {
+    DLOG(INFO) << "In " << __FUNCTION__ << " for object: " << this;
+
+    ScopedComPtr<IBindCtx> bind_context;
+    HRESULT hr = GetMarshalledBindContext(bind_context.Receive());
+    if (bind_context.get()) {
+      hr = bind_context->EnumObjectParam(enum_string);
+    }
+    return hr;
+  }
+
+  STDMETHOD(RevokeObjectParam)(LPOLESTR key) {
+    DLOG(INFO) << "In " << __FUNCTION__ << " for object: " << this;
+
+    ScopedComPtr<IBindCtx> bind_context;
+    HRESULT hr = GetMarshalledBindContext(bind_context.Receive());
+    if (bind_context.get()) {
+      hr = bind_context->RevokeObjectParam(key);
+    }
+    return hr;
+  }
+
+ private:
+  HRESULT GetMarshalledBindContext(IBindCtx** bind_context) {
+    DCHECK(bind_context != NULL);
+    DCHECK(standard_marshal_.get() != NULL);
+
+    if (!marshalled_bind_context_.get()) {
+      LARGE_INTEGER offset = {0};
+      marshaled_stream_->Seek(offset, STREAM_SEEK_SET, NULL);
+      HRESULT hr = standard_marshal_->UnmarshalInterface(
+          marshaled_stream_, __uuidof(IBindCtx),
+          reinterpret_cast<void**>(marshalled_bind_context_.Receive()));
+      if (FAILED(hr)) {
+        NOTREACHED() << __FUNCTION__
+                     << "UnmarshalInterface failed. Error:"
+                     << hr;
+        return hr;
+      }
+      DCHECK(marshalled_bind_context_.get() != NULL);
+    }
+    return marshalled_bind_context_.QueryInterface(bind_context);
+  }
+
+  ScopedComPtr<IStream> marshaled_stream_;
+  ScopedComPtr<IBindCtx> marshalled_bind_context_;
+  ScopedComPtr<IMarshal> standard_marshal_;
+};
+
 UrlmonUrlRequest::UrlmonUrlRequest()
     : pending_read_size_(0),
       status_(URLRequestStatus::FAILED, net::ERR_FAILED),
@@ -361,7 +559,7 @@ STDMETHODIMP UrlmonUrlRequest::OnDataAvailable(DWORD flags, DWORD size,
   return S_OK;
 }
 
-STDMETHODIMP UrlmonUrlRequest::OnObjectAvailable(REFIID iid, IUnknown *object) {
+STDMETHODIMP UrlmonUrlRequest::OnObjectAvailable(REFIID iid, IUnknown* object) {
   // We are calling BindToStorage on the moniker we should always get called
   // back on OnDataAvailable and should never get OnObjectAvailable
   NOTREACHED();
@@ -614,8 +812,25 @@ HRESULT UrlmonUrlRequest::ConnectToExistingMoniker(IMoniker* moniker,
   DCHECK(moniker_.get() == NULL);
   DCHECK(bind_context_.get() == NULL);
 
+  CComObject<WrappedBindContext>* bind_context = NULL;
+  HRESULT hr = CComObject<WrappedBindContext>::CreateInstance(&bind_context);
+  if (FAILED(hr)) {
+    NOTREACHED() << "Failed to instantiate wrapped bind context. Error:" << hr;
+    return hr;
+  }
+
+  bind_context->AddRef();
+  hr = bind_context->Initialize(context);
+  DCHECK(SUCCEEDED(hr));
+
+  hr = bind_context->QueryInterface(bind_context_.Receive());
+  bind_context->Release();
+
+  if (FAILED(hr)) {
+    NOTREACHED() << "Failed to QI for IBindCtx on wrapper. Error:" << hr;
+    return hr;
+  }
   moniker_ = moniker;
-  bind_context_ = context;
   set_url(WideToUTF8(url));
   return S_OK;
 }