Fix IE6 switching issues

Fixes for issues with new moniker patch.

BUG=none
TEST=covered by existing tests

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

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

6 files changed, 51 insertions, 20 deletions

diff --git a/chrome_frame/bind_status_callback_impl.cc b/chrome_frame/bind_status_callback_impl.cc
index 90da0916..91a8424 100644
--- a/chrome_frame/bind_status_callback_impl.cc
+++ b/chrome_frame/bind_status_callback_impl.cc
@@ -45,14 +45,9 @@ HRESULT BSCBImpl::AttachToBind(IBindCtx* bind_ctx) {
 
 HRESULT BSCBImpl::ReleaseBind() {
   HRESULT hr = S_OK;
-  if (delegate_ && bind_ctx_) {
-    ScopedComPtr<IBindStatusCallback> this_callback;
-    hr = ::RegisterBindStatusCallback(bind_ctx_, delegate_,
-                                      this_callback.Receive(), 0);
-    DCHECK(this_callback &&
-           (this_callback == static_cast<IBindStatusCallback*>(this)));
+  if (bind_ctx_) {
+    hr = ::RevokeBindStatusCallback(bind_ctx_, this);
   }
-
   delegate_.Release();
   bind_ctx_.Release();
   return hr;
diff --git a/chrome_frame/chrome_active_document.cc b/chrome_frame/chrome_active_document.cc
index 04d914e..dae11b3 100644
--- a/chrome_frame/chrome_active_document.cc
+++ b/chrome_frame/chrome_active_document.cc
@@ -241,7 +241,6 @@ STDMETHODIMP ChromeActiveDocument::Load(BOOL fully_avalable,
       mgr ? mgr->original_url_with_fragment() : std::wstring()));
 
   if (mgr) {
-    mgr->set_url(L"");
     mgr->set_original_url_with_fragment(L"");
   }
 
diff --git a/chrome_frame/urlmon_bind_status_callback.cc b/chrome_frame/urlmon_bind_status_callback.cc
index cb90bf2..00d465c 100644
--- a/chrome_frame/urlmon_bind_status_callback.cc
+++ b/chrome_frame/urlmon_bind_status_callback.cc
@@ -174,6 +174,9 @@ void SniffData::DetermineRendererType() {
         }
       }
     }
+    DLOG(INFO) << __FUNCTION__ << "Url: " << url_ <<
+      StringPrintf("Renderer type: %s",
+                    renderer_type_ == CHROME ? "CHROME" : "OTHER");
   }
 }
 
@@ -256,7 +259,6 @@ STDMETHODIMP BSCBStorageBind::OnDataAvailable(DWORD flags, DWORD size,
   } else {
     hr = CallbackImpl::OnDataAvailable(flags, size, format_etc, stgmed);
   }
-
   return hr;
 }
 
@@ -264,7 +266,9 @@ STDMETHODIMP BSCBStorageBind::OnStopBinding(HRESULT hresult, LPCWSTR error) {
   DLOG(INFO) << __FUNCTION__ << StringPrintf(" tid=%i",
       PlatformThread::CurrentId());
   HRESULT hr = MayPlayBack(BSCF_LASTDATANOTIFICATION);
-  return CallbackImpl::OnStopBinding(hresult, error);
+  hr = CallbackImpl::OnStopBinding(hresult, error);
+  ReleaseBind();
+  return hr;
 }
 
 // Play back the cached data to the delegate. Normally this would happen
diff --git a/chrome_frame/urlmon_moniker.cc b/chrome_frame/urlmon_moniker.cc
index fc66e9f..e3993d4 100644
--- a/chrome_frame/urlmon_moniker.cc
+++ b/chrome_frame/urlmon_moniker.cc
@@ -208,17 +208,43 @@ void MonikerPatch::Uninitialize() {
   vtable_patch::UnpatchInterfaceMethods(IMoniker_PatchInfo);
 }
 
-bool ShouldWrapCallback(IMoniker* moniker, IBindCtx* bind_context) {
+bool ShouldWrapCallback(IMoniker* moniker, REFIID iid, IBindCtx* bind_context) {
+  CComHeapPtr<WCHAR> url;
+  HRESULT hr = moniker->GetDisplayName(bind_context, NULL, &url);
+  if (!url) {
+    DLOG(INFO) << __FUNCTION__ << StringPrintf(
+        "GetDisplayName failed. Error: 0x%x", hr);
+    return false;
+  }
+
+  if (!IsEqualIID(IID_IStream, iid)) {
+    DLOG(INFO) << __FUNCTION__ << "Url: " << url <<
+        "Not wrapping: IID is not IStream.";
+    return false;
+  }
+
+  ScopedComPtr<IUnknown> our_request;
+  hr = bind_context->GetObjectParam(L"_CHROMEFRAME_REQUEST_",
+                                    our_request.Receive());
+  if (our_request) {
+    DLOG(INFO) << __FUNCTION__ << "Url: " << url <<
+        "Not wrapping: request from chrome frame.";
+    return false;
+  }
+
   NavigationManager* mgr = NavigationManager::GetThreadInstance();
   if (!mgr) {
-    DLOG(INFO) << __FUNCTION__ << "No navitation manager to wrap";
+    DLOG(INFO) << __FUNCTION__ << "Url: " << url <<
+        "No navitagion manager to wrap";
     return false;
   }
 
-  CComHeapPtr<WCHAR> url;
-  HRESULT hr = moniker->GetDisplayName(bind_context, NULL, &url);
-  DCHECK(SUCCEEDED(hr));
   bool should_wrap = mgr->IsTopLevelUrl(url);
+  if (!should_wrap) {
+    DLOG(INFO) << __FUNCTION__ << "Url: " << url <<
+        "Not wrapping: Not top level url.";
+  }
+
   return should_wrap;
 }
 
@@ -255,12 +281,11 @@ HRESULT MonikerPatch::BindToObject(IMoniker_BindToObject_Fn original,
 HRESULT MonikerPatch::BindToStorage(IMoniker_BindToStorage_Fn original,
                                     IMoniker* me, IBindCtx* bind_ctx,
                                     IMoniker* to_left, REFIID iid, void** obj) {
-  DLOG(INFO) << __FUNCTION__;
   DCHECK(to_left == NULL);
 
   HRESULT hr = S_OK;
   CComObject<BSCBStorageBind>* callback = NULL;
-  if ((IsEqualIID(IID_IStream, iid)) && ShouldWrapCallback(me, bind_ctx)) {
+  if (ShouldWrapCallback(me, iid, bind_ctx)) {
     hr = CComObject<BSCBStorageBind>::CreateInstance(&callback);
     callback->AddRef();
     hr = callback->Initialize(me, bind_ctx);
@@ -274,6 +299,5 @@ HRESULT MonikerPatch::BindToStorage(IMoniker_BindToStorage_Fn original,
   // and then it's too late.
   if ((S_OK == hr) && callback)
     callback->MayPlayBack(BSCF_LASTDATANOTIFICATION);
-
   return hr;
 }
\ No newline at end of file
diff --git a/chrome_frame/urlmon_moniker.h b/chrome_frame/urlmon_moniker.h
index 885b7fe..ff45bec 100644
--- a/chrome_frame/urlmon_moniker.h
+++ b/chrome_frame/urlmon_moniker.h
@@ -14,6 +14,7 @@
 #include "base/logging.h"
 #include "base/scoped_comptr_win.h"
 #include "base/thread_local.h"
+#include "googleurl/src/gurl.h"
 #include "chrome_frame/utils.h"
 
 // This file contains classes that are used to cache the contents of a top-level
@@ -131,7 +132,7 @@ class NavigationManager {
   // Return true if this is a URL that represents a top-level
   // document that might have to be rendered in CF.
   virtual bool IsTopLevelUrl(const wchar_t* url) {
-    return lstrcmpiW(url_.c_str(), url) == 0;
+    return GURL(url_) == GURL(url);
   }
 
   // Called from HttpNegotiatePatch::BeginningTransaction when a request is
diff --git a/chrome_frame/urlmon_url_request.cc b/chrome_frame/urlmon_url_request.cc
index 364f01d..e3cc03b 100644
--- a/chrome_frame/urlmon_url_request.cc
+++ b/chrome_frame/urlmon_url_request.cc
@@ -624,7 +624,6 @@ HRESULT UrlmonUrlRequest::StartAsyncDownload() {
   HRESULT hr = E_FAIL;
   DCHECK((moniker_ && bind_context_) || (!moniker_ && !bind_context_));
 
-
   if (!moniker_.get()) {
     std::wstring wide_url = UTF8ToWide(url());
     hr = CreateURLMonikerEx(NULL, wide_url.c_str(), moniker_.Receive(),
@@ -653,8 +652,17 @@ HRESULT UrlmonUrlRequest::StartAsyncDownload() {
     // in destruction of our object. It's fine but we access some members
     // below for debug info. :)
     ScopedComPtr<IHttpSecurity> self(this);
+
+    // Inform our moniker patch this binding should nto be tortured.
+    // TODO(amit): factor this out.
+    hr = bind_context_->RegisterObjectParam(L"_CHROMEFRAME_REQUEST_", self);
+    DCHECK(SUCCEEDED(hr));
+
     hr = moniker_->BindToStorage(bind_context_, NULL, __uuidof(IStream),
                                  reinterpret_cast<void**>(stream.Receive()));
+
+    bind_context_->RevokeObjectParam(L"_CHROMEFRAME_REQUEST_");
+
     if (hr == S_OK) {
       DCHECK(binding_ != NULL || status_.get_state() == Status::DONE);
     }