IE would not switch into ChromeFrame if the url being navigated to had an anchor. The IsTopLevelUrl function exposed by the navigation manager

would compare the URL received in the BHO which has the anchor and the URL received from the anchor which does not have the anchor. As a result
we would not wrap the bind status callback which caused this issue.

The IsTopLevelUrl function now compares the URLs after stripping the anchor portion in the URL.

Should fix bug http://code.google.com/p/chromium/issues/detail?id=38265.

This only works if the moniker patch is enabled.

Bug=38265
Test=Covered by unit test.

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

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

6 files changed, 70 insertions, 26 deletions

diff --git a/chrome_frame/chrome_active_document.cc b/chrome_frame/chrome_active_document.cc
index dae11b3..40be889 100644
--- a/chrome_frame/chrome_active_document.cc
+++ b/chrome_frame/chrome_active_document.cc
@@ -237,12 +237,7 @@ STDMETHODIMP ChromeActiveDocument::Load(BOOL fully_avalable,
   // from the moniker does not contain the anchor. To workaround
   // this we retrieve the URL from our BHO.
   std::wstring url(GetActualUrlFromMoniker(
-      moniker_name, bind_context,
-      mgr ? mgr->original_url_with_fragment() : std::wstring()));
-
-  if (mgr) {
-    mgr->set_original_url_with_fragment(L"");
-  }
+      moniker_name, bind_context, mgr ? mgr->url(): std::wstring()));
 
   // The is_new_navigation variable indicates if this a navigation initiated
   // by typing in a URL for e.g. in the IE address bar, or from Chrome by
diff --git a/chrome_frame/test/test_mock_with_web_server.cc b/chrome_frame/test/test_mock_with_web_server.cc
index fbff8fb..c777b37 100644
--- a/chrome_frame/test/test_mock_with_web_server.cc
+++ b/chrome_frame/test/test_mock_with_web_server.cc
@@ -235,7 +235,7 @@ ACTION_P3(TypeUrlInAddressBar, loop, url, delay) {
       simulate_input::SendCharA, 'd', simulate_input::ALT),
       delay);
 
-  const unsigned long kInterval = 25;
+  const unsigned long kInterval = 100;
   int next_delay = delay + kInterval;
 
   loop->PostDelayedTask(FROM_HERE, NewRunnableFunction(
@@ -1497,3 +1497,37 @@ TEST_F(ChromeFrameTestWithWebServer,
   EXPECT_EQ(1, response->post_request_count());
 }
 
+// This test validates that typing in URLs with a fragment in them switch to
+// to ChromeFrame correctly.
+TEST_F(ChromeFrameTestWithWebServer,
+       FLAKY_FullTabModeIE_AltD_AnchorUrlNavigate) {
+  if (!MonikerPatchEnabled())
+    return;
+
+  CloseIeAtEndOfScope last_resort_close_ie;
+  chrome_frame_test::TimedMsgLoop loop;
+  ComStackObjectWithUninitialize<MockWebBrowserEventSink> mock;
+  ::testing::InSequence sequence;
+
+  mock.ExpectNavigationAndSwitchSequence(kSubFrameUrl1);
+  EXPECT_CALL(mock, OnLoad(testing::StrCaseEq(kSubFrameUrl1)))
+      .WillOnce(testing::DoAll(
+          SetFocusToChrome(&mock),
+          TypeUrlInAddressBar(&loop, kAnchor1Url, 1500)));
+
+  mock.ExpectNavigationAndSwitchSequence(kAnchor1Url);
+  EXPECT_CALL(mock, OnLoad(testing::StrCaseEq(kAnchor1Url)))
+      .WillOnce(CloseBrowserMock(&mock));
+
+  EXPECT_CALL(mock, OnQuit())
+      .Times(testing::AtMost(1))
+      .WillOnce(QUIT_LOOP(loop));
+
+  HRESULT hr = mock.LaunchIEAndNavigate(kSubFrameUrl1);
+  ASSERT_HRESULT_SUCCEEDED(hr);
+  if (hr == S_FALSE)
+    return;
+
+  ASSERT_TRUE(mock.web_browser2() != NULL);
+  loop.RunFor(kChromeFrameLongNavigationTimeoutInSeconds);
+}
diff --git a/chrome_frame/urlmon_moniker.cc b/chrome_frame/urlmon_moniker.cc
index 76a5bea..49632fd 100644
--- a/chrome_frame/urlmon_moniker.cc
+++ b/chrome_frame/urlmon_moniker.cc
@@ -82,7 +82,6 @@ HRESULT NavigationManager::NavigateToCurrentUrlInCF(IBrowserService* browser) {
       GURL parsed_moniker_url(url_);
       if (parsed_moniker_url.has_ref()) {
         fragment = UTF8ToWide(parsed_moniker_url.ref());
-        set_original_url_with_fragment(url_.c_str());
       }
 
       hr = NavigateBrowserToMoniker(browser, moniker, headers.c_str(),
@@ -94,6 +93,10 @@ HRESULT NavigationManager::NavigateToCurrentUrlInCF(IBrowserService* browser) {
   return hr;
 }
 
+bool NavigationManager::IsTopLevelUrl(const wchar_t* url) {
+  return CompareUrlsWithoutFragment(url_.c_str(), url);
+}
+
 void NavigationManager::OnBeginningTransaction(bool is_top_level,
     const wchar_t* url, const wchar_t* headers,
     const wchar_t* additional_headers) {
@@ -278,4 +281,5 @@ HRESULT MonikerPatch::BindToStorage(IMoniker_BindToStorage_Fn original,
   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 fcb88fd..5cea8cf 100644
--- a/chrome_frame/urlmon_moniker.h
+++ b/chrome_frame/urlmon_moniker.h
@@ -110,15 +110,6 @@ class NavigationManager {
     url_ = url;
   }
 
-  const std::wstring& original_url_with_fragment() const {
-    return original_url_with_fragment_;
-  }
-
-  void set_original_url_with_fragment(const wchar_t* url) {
-    DLOG(INFO) << __FUNCTION__ << " " << url;
-    original_url_with_fragment_ = url;
-  }
-
   // Returns the referrer header value of the current top level navigation.
   const std::string& referrer() const {
     return referrer_;
@@ -130,9 +121,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 GURL(url_) == GURL(url);
-  }
+  virtual bool IsTopLevelUrl(const wchar_t* url);
 
   // Called from HttpNegotiatePatch::BeginningTransaction when a request is
   // being issued.  We check the url and headers and see if there is a referrer
@@ -152,11 +141,6 @@ class NavigationManager {
   static base::LazyInstance<base::ThreadLocalPointer<NavigationManager> >
       thread_singleton_;
 
-  // If the url being navigated to within ChromeFrame has a fragment, this
-  // member contains this URL. This member is cleared when the Chrome active
-  // document is loaded.
-  std::wstring original_url_with_fragment_;
-
  private:
   DISALLOW_COPY_AND_ASSIGN(NavigationManager);
 };
diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc
index d314a24..eb0c802 100644
--- a/chrome_frame/utils.cc
+++ b/chrome_frame/utils.cc
@@ -973,3 +973,22 @@ int32 MapCookieStateToCookieAction(InternetCookieState cookie_state) {
   return cookie_action;
 }
 
+GURL GetUrlWithoutFragment(const wchar_t* url) {
+  GURL parsed_url(url);
+
+  if (parsed_url.has_ref()) {
+    url_parse::Component comp;
+    GURL::Replacements replacements;
+    replacements.SetRef("", comp);
+
+    parsed_url = parsed_url.ReplaceComponents(replacements);
+  }
+  return parsed_url;
+}
+
+bool CompareUrlsWithoutFragment(const wchar_t* url1, const wchar_t* url2) {
+  GURL parsed_url1 = GetUrlWithoutFragment(url1);
+  GURL parsed_url2 = GetUrlWithoutFragment(url2);
+  return parsed_url1 == parsed_url2;
+}
+
diff --git a/chrome_frame/utils.h b/chrome_frame/utils.h
index ad5a335..b6f6dd0 100644
--- a/chrome_frame/utils.h
+++ b/chrome_frame/utils.h
@@ -19,6 +19,8 @@
 #include "base/logging.h"
 #include "base/thread.h"
 
+#include "googleurl/src/gurl.h"
+
 // utils.h : Various utility functions and classes
 
 extern const wchar_t kChromeContentPrefix[];
@@ -406,4 +408,10 @@ extern Lock g_ChromeFrameHistogramLock;
 // used for IE privacy stuff.
 int32 MapCookieStateToCookieAction(InternetCookieState cookie_state);
 
+// Parses the url passed in and returns a GURL instance without the fragment.
+GURL GetUrlWithoutFragment(const wchar_t* url);
+
+// Compares the URLs passed in after removing the fragments from them.
+bool CompareUrlsWithoutFragment(const wchar_t* url1, const wchar_t* url2);
+
 #endif  // CHROME_FRAME_UTILS_H_