Fix for referrer flakyness. There were two problems:

Grabbing the referrer header when the request is made and not in BeforeNavigate2.
The request headers aren't always available in BeforeNavigate2.  Mshtml provides these
headers in IHttpNegotiate::BeginningTransaction, which is where we now grab them.

There was a race in chrome_frame_automation.cc where we use std::string to store the referrer url and then access that string from different threads.  This was causing the referrer URL to be missing at some times and at other times be sent over to chrome as bad string (e.g. containing embedded nulls) and subsequently be deemed an invalid URL and dropped.

TEST=Fixes referrer header flakyness.
BUG=34812

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

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

5 files changed, 74 insertions, 28 deletions

diff --git a/chrome_frame/bho.cc b/chrome_frame/bho.cc
index aee9282..a142727 100644
--- a/chrome_frame/bho.cc
+++ b/chrome_frame/bho.cc
@@ -100,32 +100,17 @@ STDMETHODIMP Bho::BeforeNavigate2(IDispatch* dispatch, VARIANT* url,
 
   if (!web_browser2 || url->vt != VT_BSTR) {
     NOTREACHED() << "Can't find WebBrowser2 with given dispatch";
-    return S_OK;  // Return success, we operate on best effort basis.
-  }
-
-  DLOG(INFO) << "BeforeNavigate2: " << url->bstrVal;
-
-  ProcessOptInUrls(web_browser2, url->bstrVal);
-
-  referrer_.clear();
-
-  // Save away the referrer in case our active document needs it to initiate
-  // navigation in chrome.
-  if (headers && V_VT(headers) == VT_BSTR && headers->bstrVal != NULL) {
-    std::string raw_headers_utf8 = WideToUTF8(headers->bstrVal);
-    std::string http_headers =
-        net::HttpUtil::AssembleRawHeaders(raw_headers_utf8.c_str(),
-                                          raw_headers_utf8.length());
-    net::HttpUtil::HeadersIterator it(http_headers.begin(), http_headers.end(),
-                                      "\r\n");
-    while (it.GetNext()) {
-      if (LowerCaseEqualsASCII(it.name(), "referer")) {
-        referrer_ = it.values();
-        break;
-      }
+  } else {
+    DLOG(INFO) << "BeforeNavigate2: " << url->bstrVal;
+    ScopedComPtr<IBrowserService> browser_service;
+    DoQueryService(SID_SShellBrowser, web_browser2, browser_service.Receive());
+    if (!browser_service || !CheckForCFNavigation(browser_service, false)) {
+      referrer_.clear();
     }
+    url_ = url->bstrVal;
+    ProcessOptInUrls(web_browser2, url->bstrVal);
   }
-  url_ = url->bstrVal;
+
   return S_OK;
 }
 
@@ -272,6 +257,46 @@ HRESULT Bho::OnHttpEquiv(IBrowserService_OnHttpEquiv_Fn original_httpequiv,
   return original_httpequiv(browser, shell_view, done, in_arg, out_arg);
 }
 
+void Bho::OnBeginningTransaction(IWebBrowser2* browser, const wchar_t* url,
+                                 const wchar_t* headers,
+                                 const wchar_t* additional_headers) {
+  if (!browser)
+    return;
+
+  if (url_.compare(url) != 0) {
+    DLOG(INFO) << __FUNCTION__ << " not processing headers for url: " << url
+        << " current url is: " << url_;
+    return;
+  }
+
+  VARIANT_BOOL is_top_level = VARIANT_FALSE;
+  browser->get_TopLevelContainer(&is_top_level);
+  if (is_top_level) {
+    ScopedComPtr<IBrowserService> browser_service;
+    DoQueryService(SID_SShellBrowser, browser, browser_service.Receive());
+    if (!browser_service || !CheckForCFNavigation(browser_service, false)) {
+      // Save away the referrer in case our active document needs it to initiate
+      // navigation in chrome.
+      referrer_.clear();
+      const wchar_t* both_headers[] = { headers, additional_headers };
+      for (int i = 0; referrer_.empty() && i < arraysize(both_headers); ++i) {
+        std::string raw_headers_utf8 = WideToUTF8(both_headers[i]);
+        std::string http_headers =
+            net::HttpUtil::AssembleRawHeaders(raw_headers_utf8.c_str(),
+                                              raw_headers_utf8.length());
+        net::HttpUtil::HeadersIterator it(http_headers.begin(),
+                                          http_headers.end(), "\r\n");
+        while (it.GetNext()) {
+          if (LowerCaseEqualsASCII(it.name(), "referer")) {
+            referrer_ = it.values();
+            break;
+          }
+        }
+      }
+    }
+  }
+}
+
 Bho* Bho::GetCurrentThreadBhoInstance() {
   DCHECK(bho_current_thread_instance_.Pointer()->Get() != NULL);
   return bho_current_thread_instance_.Pointer()->Get();
diff --git a/chrome_frame/bho.h b/chrome_frame/bho.h
index 9c058ff..c82f000 100644
--- a/chrome_frame/bho.h
+++ b/chrome_frame/bho.h
@@ -100,6 +100,13 @@ END_SINK_MAP()
     return 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.
+  void OnBeginningTransaction(IWebBrowser2* browser, const wchar_t* url,
+                              const wchar_t* headers,
+                              const wchar_t* additional_headers);
+
   // Returns the Bho instance for the current thread. This is returned from
   // TLS.
   static Bho* GetCurrentThreadBhoInstance();
diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc
index 5775d3b..746f025 100644
--- a/chrome_frame/chrome_frame_automation.cc
+++ b/chrome_frame/chrome_frame_automation.cc
@@ -544,8 +544,13 @@ bool ChromeFrameAutomationClient::InitiateNavigation(
     return false;
   }
 
+  // Important: Since we will be using the referrer_ variable from a different
+  // thread, we need to force a new std::string buffer instance for the
+  // referrer_ GURL variable.  Otherwise we can run into strangeness when the
+  // GURL is accessed and it could result in a bad URL that can cause the
+  // referrer to be dropped or something worse.
+  referrer_ = GURL(referrer.c_str());
   url_ = parsed_url;
-  referrer_ = GURL(referrer);
   navigate_after_initialization_ = false;
 
   if (is_initialized()) {
diff --git a/chrome_frame/http_negotiate.cc b/chrome_frame/http_negotiate.cc
index b8a1b9a..45409f5 100644
--- a/chrome_frame/http_negotiate.cc
+++ b/chrome_frame/http_negotiate.cc
@@ -12,6 +12,7 @@
 #include "base/scoped_ptr.h"
 #include "base/string_util.h"
 
+#include "chrome_frame/bho.h"
 #include "chrome_frame/html_utils.h"
 #include "chrome_frame/urlmon_url_request.h"
 #include "chrome_frame/utils.h"
@@ -203,6 +204,15 @@ HRESULT HttpNegotiatePatch::BeginningTransaction(
     return hr;
   }
 
+  ScopedComPtr<IWebBrowser2> browser2;
+  DoQueryService(IID_ITargetFrame2, me, browser2.Receive());
+  if (browser2) {
+    Bho* bho = Bho::GetCurrentThreadBhoInstance();
+    if (bho) {
+      bho->OnBeginningTransaction(browser2, url, headers, *additional_headers);
+    }
+  }
+
   static const char kLowerCaseUserAgent[] = "user-agent";
 
   using net::HttpUtil;
diff --git a/chrome_frame/test/test_with_web_server.cc b/chrome_frame/test/test_with_web_server.cc
index 56acce1..fbd21d3 100644
--- a/chrome_frame/test/test_with_web_server.cc
+++ b/chrome_frame/test/test_with_web_server.cc
@@ -683,9 +683,8 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_NavigateOut) {
 }
 
 const wchar_t kReferrerMainTest[] = L"files/referrer_main.html";
-// Marking this as FLAKY as this has been failing randomly on the builder.
-// http://code.google.com/p/chromium/issues/detail?id=34812
-TEST_F(ChromeFrameTestWithWebServer, FLAKY_FullTabModeIE_ReferrerTest) {
+
+TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_ReferrerTest) {
   SimpleBrowserTest(IE, kReferrerMainTest, L"FullTab_ReferrerTest");
 }