Renabling the FullTabModeIE_ChromeFrameUnloadEventTest ChromeFrame test. This test failed at times

on the builder due to a deadlock between the urlmon worker thread in IE and the ui thread. This would
occur when the active document instance was going away while the worker thread was waiting for a COM
call to be marshaled to the UI thread which expects the thread to pump messages.

The only interface which needs to be marshaled to the worker thread is the IBindCtx interface. Turns out
that there is nothing of note in this object, except the bind options. The rest of the properties queried
for all fail initially and then the caller register its own variants of these properties.

Fix is to create a new IBindCtx interface and copy over the BIND_OPTs to this interface.

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


Bug=34246

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

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

4 files changed, 21 insertions, 215 deletions

diff --git a/chrome_frame/test/chrome_frame_unittests.cc b/chrome_frame/test/chrome_frame_unittests.cc
index 08228cb..f343ff2c 100644
--- a/chrome_frame/test/chrome_frame_unittests.cc
+++ b/chrome_frame/test/chrome_frame_unittests.cc
@@ -1880,10 +1880,8 @@ const wchar_t kChromeFrameFullTabModeBeforeUnloadEventTest[] =
 const wchar_t kChromeFrameFullTabModeBeforeUnloadEventMain[] =
     L"http://localhost:1337/files/fulltab_before_unload_event_main.html";
 
-// Disabled as it hangs on the builder.
-// http://code.google.com/p/chromium/issues/detail?id=34246
 TEST_F(ChromeFrameTestWithWebServer,
-       DISABLED_FullTabModeIE_ChromeFrameUnloadEventTest) {
+       FullTabModeIE_ChromeFrameUnloadEventTest) {
   chrome_frame_test::TimedMsgLoop loop;
   CComObjectStackEx<MockWebBrowserEventSink> mock;
   ::testing::InSequence sequence;   // Everything in sequence
diff --git a/chrome_frame/urlmon_url_request.cc b/chrome_frame/urlmon_url_request.cc
index 735d3f5..0fbe54b 100644
--- a/chrome_frame/urlmon_url_request.cc
+++ b/chrome_frame/urlmon_url_request.cc
@@ -22,205 +22,6 @@
 static const LARGE_INTEGER kZero = {0};
 static const ULARGE_INTEGER kUnsignedZero = {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 << " key: "
-        << key;
-
-    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 << " key: "
-        << key;
-
-    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_;
-};
-
 STDMETHODIMP UrlmonUrlRequest::SendStream::Write(const void * buffer,
                                                  ULONG size,
                                                  ULONG* size_written) {
@@ -321,9 +122,9 @@ bool UrlmonUrlRequest::Read(int bytes_to_read) {
 }
 
 HRESULT UrlmonUrlRequest::ConnectToExistingMoniker(IMoniker* moniker,
-                                                   IBindCtx* context,
+                                                   BIND_OPTS* bind_opts,
                                                    const std::wstring& url) {
-  if (!moniker || url.empty()) {
+  if (!moniker || url.empty() || !bind_opts) {
     NOTREACHED() << "Invalid arguments";
     return E_INVALIDARG;
   }
@@ -331,7 +132,13 @@ HRESULT UrlmonUrlRequest::ConnectToExistingMoniker(IMoniker* moniker,
   DCHECK(moniker_.get() == NULL);
   DCHECK(bind_context_.get() == NULL);
 
-  bind_context_ = context;
+  HRESULT hr = CreateAsyncBindCtx(0, this, NULL, bind_context_.Receive());
+  if (FAILED(hr)) {
+    NOTREACHED() << "Failed to create bind context";
+    return hr;
+  }
+
+  bind_context_->SetBindOptions(bind_opts);
   moniker_ = moniker;
   set_url(WideToUTF8(url));
   return S_OK;
@@ -805,7 +612,7 @@ HRESULT UrlmonUrlRequest::StartAsyncDownload() {
     }
   } else {
     DCHECK(bind_context_.get() != NULL);
-    hr = RegisterBindStatusCallback(bind_context_, this, NULL, 0);
+    hr = S_OK;
   }
 
   if (SUCCEEDED(hr)) {
@@ -1020,15 +827,12 @@ void UrlmonUrlRequestManager::UseMonikerForUrl(IMoniker* moniker,
                                                IBindCtx* bind_ctx,
                                                const std::wstring& url) {
   DCHECK(NULL == moniker_for_url_.get());
+  DCHECK(bind_ctx != NULL);
+
   moniker_for_url_.reset(new MonikerForUrl());
+  bind_ctx->GetBindOptions(&moniker_for_url_->bind_opts);
   moniker_for_url_->moniker = moniker;
   moniker_for_url_->url = url;
-
-  CComObject<WrappedBindContext>* ctx = NULL;
-  CComObject<WrappedBindContext>::CreateInstance(&ctx);
-  ctx->Initialize(bind_ctx);
-  ctx->QueryInterface(moniker_for_url_->bind_ctx.Receive());
-  DCHECK(moniker_for_url_->bind_ctx.get());
 }
 
 void UrlmonUrlRequestManager::StartRequest(int request_id,
@@ -1078,7 +882,7 @@ void UrlmonUrlRequestManager::StartRequestWorker(int request_id,
   // Shall we use an existing moniker?
   if (moniker_for_url.get()) {
     new_request->ConnectToExistingMoniker(moniker_for_url->moniker,
-                                          moniker_for_url->bind_ctx,
+                                          &moniker_for_url->bind_opts,
                                           moniker_for_url->url);
   }
 
diff --git a/chrome_frame/urlmon_url_request.h b/chrome_frame/urlmon_url_request.h
index 325edb8..894a431 100644
--- a/chrome_frame/urlmon_url_request.h
+++ b/chrome_frame/urlmon_url_request.h
@@ -34,8 +34,11 @@ class UrlmonUrlRequestManager :
 
  private:
    struct MonikerForUrl {
+     MonikerForUrl() {
+       memset(&bind_opts, 0, sizeof(bind_opts));
+     }
      ScopedComPtr<IMoniker> moniker;
-     ScopedComPtr<IBindCtx> bind_ctx;
+     BIND_OPTS bind_opts;
      std::wstring url;
    };
 
diff --git a/chrome_frame/urlmon_url_request_private.h b/chrome_frame/urlmon_url_request_private.h
index 9b42904..3434ed7 100644
--- a/chrome_frame/urlmon_url_request_private.h
+++ b/chrome_frame/urlmon_url_request_private.h
@@ -29,7 +29,8 @@ class UrlmonUrlRequest
   virtual bool Read(int bytes_to_read);
 
   // Special function needed by ActiveDocument::Load()
-  HRESULT ConnectToExistingMoniker(IMoniker* moniker, IBindCtx* context,
+  HRESULT ConnectToExistingMoniker(IMoniker* moniker,
+                                   BIND_OPTS* bind_opts,
                                    const std::wstring& url);
 
   // Used from "OnDownloadRequestInHost".