diff options
author | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-04-16 01:10:47 +0000 |
---|---|---|
committer | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-04-16 01:10:47 +0000 |
commit | 6ee88534895ff56a131400acfd2a837824ef1ead (patch) | |
tree | dda29859ab5e7ee86fc80afdc1f3dca3ac86438d /chrome_frame | |
parent | 6044824e6c218ea5c801934ac52b88d3b10f89eb (diff) | |
download | chromium_src-6ee88534895ff56a131400acfd2a837824ef1ead.zip chromium_src-6ee88534895ff56a131400acfd2a837824ef1ead.tar.gz chromium_src-6ee88534895ff56a131400acfd2a837824ef1ead.tar.bz2 |
With the ChromeFrame IMoniker patch in the referrer would not propagate correctly to Chrome when we switch from IE to CF. In ChromeFrame
the referrer is set in the navigation manager which receives this in the IHttpNegotiate::BeginningTransaction patch. When we switch from IE
to cF via the moniker patch we receive two calls to BeginningTransaction, the first one with the referrer and the other one without the
referrer for the same URL causing the referrer to be overwritten.
Fix is to handle this case. The referrer is cleared in our BeforeNavigate notification.
I also moved some functions to chrome frame utils as part of this CL.
Fixes bug http://code.google.com/p/chromium/issues/detail?id=41680
Bug=41680
Review URL: http://codereview.chromium.org/1653006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@44733 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome_frame')
-rw-r--r-- | chrome_frame/bho.cc | 1 | ||||
-rw-r--r-- | chrome_frame/http_negotiate.cc | 31 | ||||
-rw-r--r-- | chrome_frame/test/test_with_web_server.cc | 3 | ||||
-rw-r--r-- | chrome_frame/urlmon_moniker.cc | 52 | ||||
-rw-r--r-- | chrome_frame/urlmon_moniker.h | 7 | ||||
-rw-r--r-- | chrome_frame/urlmon_url_request.cc | 49 | ||||
-rw-r--r-- | chrome_frame/urlmon_url_request_private.h | 2 | ||||
-rw-r--r-- | chrome_frame/utils.cc | 65 | ||||
-rw-r--r-- | chrome_frame/utils.h | 10 |
9 files changed, 103 insertions, 117 deletions
diff --git a/chrome_frame/bho.cc b/chrome_frame/bho.cc index 3a731bc..2877179 100644 --- a/chrome_frame/bho.cc +++ b/chrome_frame/bho.cc @@ -120,6 +120,7 @@ STDMETHODIMP Bho::BeforeNavigate2(IDispatch* dispatch, VARIANT* url, web_browser2->get_TopLevelContainer(&is_top_level); if (is_top_level) { set_url(url->bstrVal); + set_referrer(""); ProcessOptInUrls(web_browser2, url->bstrVal); } diff --git a/chrome_frame/http_negotiate.cc b/chrome_frame/http_negotiate.cc index a627ad8..a95255a 100644 --- a/chrome_frame/http_negotiate.cc +++ b/chrome_frame/http_negotiate.cc @@ -207,15 +207,28 @@ HRESULT HttpNegotiatePatch::BeginningTransaction( ScopedComPtr<IWebBrowser2> browser2; DoQueryService(IID_ITargetFrame2, me, browser2.Receive()); if (browser2) { - NavigationManager* mgr = NavigationManager::GetThreadInstance(); - if (mgr) { - VARIANT_BOOL is_top_level = VARIANT_FALSE; - browser2->get_TopLevelContainer(&is_top_level); - mgr->OnBeginningTransaction(is_top_level != VARIANT_FALSE, url, headers, - *additional_headers); - DLOG(INFO) << "called OnBeginningTransaction " << is_top_level; - } else { - DLOG(INFO) << "No NavigationManager"; + VARIANT_BOOL is_top_level = VARIANT_FALSE; + browser2->get_TopLevelContainer(&is_top_level); + + DLOG(INFO) << "called OnBeginningTransaction " << is_top_level; + + if (is_top_level != VARIANT_FALSE) { + std::string referrer = FindReferrerFromHeaders(headers, + *additional_headers); + NavigationManager* mgr = NavigationManager::GetThreadInstance(); + if (mgr) { + // When we switch from IE to CF the BeginningTransaction function is + // called twice. The first call contains the referrer while the + // second call does not. We set the referrer only if the URL in the + // navigation manager changes. The URL in the navigation manager + // is reset in BeforeNavigate2 + if (!referrer.empty()) { + DCHECK(mgr->referrer().empty()); + mgr->set_referrer(referrer); + } + } else { + DLOG(INFO) << "No NavigationManager"; + } } } else { DLOG(INFO) << "No IWebBrowser2"; diff --git a/chrome_frame/test/test_with_web_server.cc b/chrome_frame/test/test_with_web_server.cc index 481716f..b5c9648 100644 --- a/chrome_frame/test/test_with_web_server.cc +++ b/chrome_frame/test/test_with_web_server.cc @@ -703,8 +703,7 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_NavigateOut) { const wchar_t kReferrerMainTest[] = L"files/referrer_main.html"; -// Disabled temporarily: http://crbug.com/41680 -TEST_F(ChromeFrameTestWithWebServer, DISABLED_FullTabModeIE_ReferrerTest) { +TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_ReferrerTest) { SimpleBrowserTest(IE, kReferrerMainTest, L"FullTab_ReferrerTest"); } diff --git a/chrome_frame/urlmon_moniker.cc b/chrome_frame/urlmon_moniker.cc index f73b8af..4713da0 100644 --- a/chrome_frame/urlmon_moniker.cc +++ b/chrome_frame/urlmon_moniker.cc @@ -11,6 +11,7 @@ #include "chrome_frame/bind_context_info.h" #include "chrome_frame/chrome_active_document.h" #include "chrome_frame/urlmon_bind_status_callback.h" +#include "chrome_frame/utils.h" #include "chrome_frame/vtable_patch_manager.h" #include "net/http/http_util.h" @@ -25,32 +26,6 @@ BEGIN_VTABLE_PATCHES(IMoniker) VTABLE_PATCH_ENTRY(kMonikerBindToStorage, MonikerPatch::BindToStorage) END_VTABLE_PATCHES() -namespace { -std::string FindReferrerFromHeaders(const wchar_t* headers, - const wchar_t* additional_headers) { - std::string referrer; - - const wchar_t* both_headers[] = { headers, additional_headers }; - for (int i = 0; referrer.empty() && i < arraysize(both_headers); ++i) { - if (!both_headers[i]) - continue; - std::string raw_headers_utf8 = WideToUTF8(both_headers[i]); - net::HttpUtil::HeadersIterator it(raw_headers_utf8.begin(), - raw_headers_utf8.end(), "\r\n"); - while (it.GetNext()) { - if (LowerCaseEqualsASCII(it.name(), "referer")) { - referrer = it.values(); - break; - } - } - } - - return referrer; -} - -} // end namespace - - //////////////////////////// HRESULT NavigationManager::NavigateToCurrentUrlInCF(IBrowserService* browser) { @@ -97,31 +72,6 @@ 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) { - DCHECK(url != NULL); - - // We've seen this happen on the first page load in IE8 (it might also happen - // in other versions of IE) and for some reason we did not get a call to - // BeginningTransaction the first time it happened and this time around we're - // using a cached data object that didn't get the request headers or URL. - DLOG_IF(ERROR, lstrlenW(url) == 0) << __FUNCTION__ << "Invalid URL."; - - if (!is_top_level) - return; - - if (url_.compare(url) != 0) { - DLOG(INFO) << __FUNCTION__ << " not processing headers for url: " << url - << " current url is: " << url_; - return; - } - - // Save away the referrer in case our active document needs it to initiate - // navigation in chrome. - referrer_ = FindReferrerFromHeaders(headers, additional_headers); -} - ///////////////////////////////////////// NavigationManager* NavigationManager::GetThreadInstance() { diff --git a/chrome_frame/urlmon_moniker.h b/chrome_frame/urlmon_moniker.h index 7aa36d8..1f168a8 100644 --- a/chrome_frame/urlmon_moniker.h +++ b/chrome_frame/urlmon_moniker.h @@ -119,13 +119,6 @@ class NavigationManager { // document that might have to be rendered in CF. 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 - // header that we need to cache. - virtual void OnBeginningTransaction(bool is_top_level, const wchar_t* url, - const wchar_t* headers, - const wchar_t* additional_headers); - // Called when we've detected the http-equiv meta tag in the current page // and need to switch over from mshtml to CF. virtual HRESULT NavigateToCurrentUrlInCF(IBrowserService* browser); diff --git a/chrome_frame/urlmon_url_request.cc b/chrome_frame/urlmon_url_request.cc index 3451430..5c9ba42 100644 --- a/chrome_frame/urlmon_url_request.cc +++ b/chrome_frame/urlmon_url_request.cc @@ -202,7 +202,7 @@ STDMETHODIMP UrlmonUrlRequest::OnProgress(ULONG progress, ULONG max_progress, DLOG(INFO) << "URL: " << url() << " redirected to " << status_text; // Fetch the redirect status as they aren't all equal (307 in particular // retains the HTTP request verb). - int http_code = GetHttpResponseStatus(); + int http_code = GetHttpResponseStatusFromBinding(binding_); status_.SetRedirected(http_code, WideToUTF8(status_text)); // Abort. We will inform Chrome in OnStopBinding callback. binding_->Abort(); @@ -270,7 +270,7 @@ STDMETHODIMP UrlmonUrlRequest::OnStopBinding(HRESULT result, LPCWSTR error) { // attempted. In that case, correct the OS error value to be the more // specific ERR_UNSAFE_REDIRECT error value. if (result == E_ACCESSDENIED) { - int http_code = GetHttpResponseStatus(); + int http_code = GetHttpResponseStatusFromBinding(binding_); if (300 <= http_code && http_code < 400) { status_.set_result(URLRequestStatus::FAILED, net::ERR_UNSAFE_REDIRECT); @@ -309,7 +309,7 @@ STDMETHODIMP UrlmonUrlRequest::OnStopBinding(HRESULT result, LPCWSTR error) { if (status_.was_redirected()) { // Just release bindings here. Chrome will issue EndRequest(request_id) // after processing headers we had provided. - std::string headers = GetHttpHeaders(); + std::string headers = GetHttpHeadersFromBinding(binding_); OnResponse(0, UTF8ToWide(headers).c_str(), NULL, NULL); ReleaseBindings(); return S_OK; @@ -722,49 +722,6 @@ void UrlmonUrlRequest::NotifyDelegateAndDie() { } } -int UrlmonUrlRequest::GetHttpResponseStatus() const { - DLOG(INFO) << __FUNCTION__; - if (binding_ == NULL) { - DLOG(WARNING) << "GetHttpResponseStatus - no binding_"; - return 0; - } - - int http_status = 0; - - ScopedComPtr<IWinInetHttpInfo> info; - if (SUCCEEDED(info.QueryFrom(binding_))) { - char status[10] = {0}; - DWORD buf_size = sizeof(status); - DWORD flags = 0; - DWORD reserved = 0; - if (SUCCEEDED(info->QueryInfo(HTTP_QUERY_STATUS_CODE, status, &buf_size, - &flags, &reserved))) { - http_status = StringToInt(status); - } else { - NOTREACHED() << "Failed to get HTTP status"; - } - } else { - NOTREACHED() << "failed to get IWinInetHttpInfo from binding_"; - } - - return http_status; -} - -std::string UrlmonUrlRequest::GetHttpHeaders() const { - if (binding_ == NULL) { - DLOG(WARNING) << "GetHttpResponseStatus - no binding_"; - return std::string(); - } - - ScopedComPtr<IWinInetHttpInfo> info; - if (FAILED(info.QueryFrom(binding_))) { - DLOG(WARNING) << "Failed to QI for IWinInetHttpInfo"; - return std::string(); - } - - return GetRawHttpHeaders(info); -} - void UrlmonUrlRequest::ReleaseBindings() { binding_.Release(); // Do not release bind_context here! diff --git a/chrome_frame/urlmon_url_request_private.h b/chrome_frame/urlmon_url_request_private.h index 589ef93..ad7aa5a 100644 --- a/chrome_frame/urlmon_url_request_private.h +++ b/chrome_frame/urlmon_url_request_private.h @@ -159,8 +159,6 @@ class UrlmonUrlRequest HRESULT StartAsyncDownload(); void NotifyDelegateAndDie(); - int GetHttpResponseStatus() const; - std::string GetHttpHeaders() const; static net::Error HresultToNetError(HRESULT hr); private: diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc index eb0c802..1bc13ac 100644 --- a/chrome_frame/utils.cc +++ b/chrome_frame/utils.cc @@ -29,6 +29,7 @@ #include "googleurl/src/url_canon.h" #include "grit/chromium_strings.h" +#include "net/http/http_util.h" // Note that these values are all lower case and are compared to // lower-case-transformed values. @@ -992,3 +993,67 @@ bool CompareUrlsWithoutFragment(const wchar_t* url1, const wchar_t* url2) { return parsed_url1 == parsed_url2; } +std::string FindReferrerFromHeaders(const wchar_t* headers, + const wchar_t* additional_headers) { + std::string referrer; + + const wchar_t* both_headers[] = { headers, additional_headers }; + for (int i = 0; referrer.empty() && i < arraysize(both_headers); ++i) { + if (!both_headers[i]) + continue; + std::string raw_headers_utf8 = WideToUTF8(both_headers[i]); + net::HttpUtil::HeadersIterator it(raw_headers_utf8.begin(), + raw_headers_utf8.end(), "\r\n"); + while (it.GetNext()) { + if (LowerCaseEqualsASCII(it.name(), "referer")) { + referrer = it.values(); + break; + } + } + } + + return referrer; +} + +std::string GetHttpHeadersFromBinding(IBinding* binding) { + if (binding == NULL) { + DLOG(WARNING) << "GetHttpResponseStatus - no binding_"; + return std::string(); + } + + ScopedComPtr<IWinInetHttpInfo> info; + if (FAILED(info.QueryFrom(binding))) { + DLOG(WARNING) << "Failed to QI for IWinInetHttpInfo"; + return std::string(); + } + + return GetRawHttpHeaders(info); +} + +int GetHttpResponseStatusFromBinding(IBinding* binding) { + DLOG(INFO) << __FUNCTION__; + if (binding == NULL) { + DLOG(WARNING) << "GetHttpResponseStatus - no binding_"; + return 0; + } + + int http_status = 0; + + ScopedComPtr<IWinInetHttpInfo> info; + if (SUCCEEDED(info.QueryFrom(binding))) { + char status[10] = {0}; + DWORD buf_size = sizeof(status); + DWORD flags = 0; + DWORD reserved = 0; + if (SUCCEEDED(info->QueryInfo(HTTP_QUERY_STATUS_CODE, status, &buf_size, + &flags, &reserved))) { + http_status = StringToInt(status); + } else { + NOTREACHED() << "Failed to get HTTP status"; + } + } else { + NOTREACHED() << "failed to get IWinInetHttpInfo from binding_"; + } + + return http_status; +} diff --git a/chrome_frame/utils.h b/chrome_frame/utils.h index b6f6dd0..5591ff09 100644 --- a/chrome_frame/utils.h +++ b/chrome_frame/utils.h @@ -414,4 +414,14 @@ 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); +// Returns the Referrer from the HTTP headers and additional headers. +std::string FindReferrerFromHeaders(const wchar_t* headers, + const wchar_t* additional_headers); + +// Returns the HTTP headers from the binding passed in. +std::string GetHttpHeadersFromBinding(IBinding* binding); + +// Returns the HTTP response code from the binding passed in. +int GetHttpResponseStatusFromBinding(IBinding* binding); + #endif // CHROME_FRAME_UTILS_H_ |