Limit the X-UA-Compatible HTTP header-based altering of the mime type performed by Chrome Frame to top-level requests only in IE.

BUG=having an iframe that requests a resource that includes the X-UA-Compatible header in the response will trigger CF taking over the page.
TEST=BUG doesn't happen anymore.

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

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

4 files changed, 88 insertions, 65 deletions

diff --git a/chrome_frame/http_negotiate.cc b/chrome_frame/http_negotiate.cc
index 5f89a76..ef996e5 100644
--- a/chrome_frame/http_negotiate.cc
+++ b/chrome_frame/http_negotiate.cc
@@ -234,34 +234,38 @@ HRESULT HttpNegotiatePatch::ReportProgress(
   if (status_code == BINDSTATUS_MIMETYPEAVAILABLE ||
       status_code == BINDSTATUS_VERIFIEDMIMETYPEAVAILABLE ||
       status_code == LOCAL_BINDSTATUS_SERVER_MIMETYPEAVAILABLE) {
-    // Check to see if we need to alter the mime type that gets reported
-    // by inspecting the raw header information:
-    ScopedComPtr<IWinInetHttpInfo> win_inet_http_info;
-    HRESULT hr = win_inet_http_info.QueryFrom(me);
-
-    // Try slightly harder if we couldn't QI directly.
-    if (!win_inet_http_info || FAILED(hr)) {
-      hr = DoQueryService(IID_IWinInetHttpInfo, me,
-                          win_inet_http_info.Receive());
-    }
+    bool is_top_level_request = !IsSubFrameRequest(me);
+
+    if (is_top_level_request) {
+      // Check to see if we need to alter the mime type that gets reported
+      // by inspecting the raw header information:
+      ScopedComPtr<IWinInetHttpInfo> win_inet_http_info;
+      HRESULT hr = win_inet_http_info.QueryFrom(me);
+
+      // Try slightly harder if we couldn't QI directly.
+      if (!win_inet_http_info || FAILED(hr)) {
+        hr = DoQueryService(IID_IWinInetHttpInfo, me,
+                            win_inet_http_info.Receive());
+      }
 
-    // Note that it has been observed that getting an IWinInetHttpInfo will
-    // fail if we are loading a page like about:blank that isn't loaded via
-    // wininet.
-    if (win_inet_http_info) {
-      // We have headers: check to see if the server is requesting CF via
-      // the X-UA-Compatible: chrome=1 HTTP header.
-      std::string headers(GetRawHttpHeaders(win_inet_http_info));
-      if (net::HttpUtil::HasHeader(headers, kUACompatibleHttpHeader)) {
-        net::HttpUtil::HeadersIterator it(headers.begin(), headers.end(),
-                                          "\r\n");
-        while (it.GetNext()) {
-          if (LowerCaseEqualsASCII(it.name_begin(), it.name_end(),
-                                   kUACompatibleHttpHeader)) {
-            std::string ua_value(StringToLowerASCII(it.values()));
-            if (ua_value.find("chrome=1") != std::string::npos) {
-              status_text = kChromeMimeType;
-              break;
+      // Note that it has been observed that getting an IWinInetHttpInfo will
+      // fail if we are loading a page like about:blank that isn't loaded via
+      // wininet.
+      if (win_inet_http_info) {
+        // We have headers: check to see if the server is requesting CF via
+        // the X-UA-Compatible: chrome=1 HTTP header.
+        std::string headers(GetRawHttpHeaders(win_inet_http_info));
+        if (net::HttpUtil::HasHeader(headers, kUACompatibleHttpHeader)) {
+          net::HttpUtil::HeadersIterator it(headers.begin(), headers.end(),
+                                            "\r\n");
+          while (it.GetNext()) {
+            if (LowerCaseEqualsASCII(it.name_begin(), it.name_end(),
+                                     kUACompatibleHttpHeader)) {
+                std::string ua_value(StringToLowerASCII(it.values()));
+                if (ua_value.find("chrome=1") != std::string::npos) {
+                  status_text = kChromeMimeType;
+                  break;
+                }
             }
           }
         }
@@ -269,6 +273,5 @@ HRESULT HttpNegotiatePatch::ReportProgress(
     }
   }
 
-  HRESULT hr = original(me, status_code, status_text);
-  return hr;
-}
\ No newline at end of file
+  return original(me, status_code, status_text);
+}
diff --git a/chrome_frame/protocol_sink_wrap.cc b/chrome_frame/protocol_sink_wrap.cc
index 9888209..d4498299 100644
--- a/chrome_frame/protocol_sink_wrap.cc
+++ b/chrome_frame/protocol_sink_wrap.cc
@@ -70,10 +70,13 @@ ProtocolSinkWrap::ProtocolSinkWrap()
 }
 
 ProtocolSinkWrap::~ProtocolSinkWrap() {
-  CComCritSecLock<CComAutoCriticalSection> lock(sink_map_lock_);
-  DCHECK(sink_map_.end() != sink_map_.find(protocol_));
-  sink_map_.erase(protocol_);
-  protocol_ = NULL;
+  // This object may be destroyed before Initialize is called.
+  if (protocol_ != NULL) {
+    CComCritSecLock<CComAutoCriticalSection> lock(sink_map_lock_);
+    DCHECK(sink_map_.end() != sink_map_.find(protocol_));
+    sink_map_.erase(protocol_);
+    protocol_ = NULL;
+  }
   DLOG(INFO) << "ProtocolSinkWrap: active sinks: " << sink_map_.size();
 }
 
@@ -614,47 +617,23 @@ ScopedComPtr<IInternetProtocolSink> ProtocolSinkWrap::MaybeWrapSink(
     IInternetProtocol* protocol, IInternetProtocolSink* prot_sink,
     const wchar_t* url) {
   ScopedComPtr<IInternetProtocolSink> sink_to_use(prot_sink);
-  ScopedComPtr<IWebBrowser2> web_browser;
 
   // FYI: GUID_NULL doesn't work when the URL is being loaded from history.
   // asking for IID_IHttpNegotiate as the service id works, but
   // getting the IWebBrowser2 interface still doesn't work.
   ScopedComPtr<IHttpNegotiate> http_negotiate;
   HRESULT hr = DoQueryService(GUID_NULL, prot_sink, http_negotiate.Receive());
-  if (http_negotiate) {
-    hr = DoQueryService(IID_ITargetFrame2, http_negotiate,
-                        web_browser.Receive());
-  }
-
-  if (web_browser) {
-    // Do one more check to make sure we don't wrap requests that are
-    // targeted to sub frames.
-    // For a use case, see FullTabModeIE_SubIFrame and FullTabModeIE_SubFrame
-    // unit tests.
-
-    // Default should_wrap to true in case no window is available.
-    // In that case this request is a top level request.
-    bool should_wrap = true;
-
-    ScopedComPtr<IHTMLWindow2> current_frame, parent_frame;
-    hr = DoQueryService(IID_IHTMLWindow2, http_negotiate,
-                        current_frame.Receive());
-    if (current_frame) {
-      // Only the top level window will return self when get_parent is called.
-      current_frame->get_parent(parent_frame.Receive());
-      if (parent_frame != current_frame) {
-        DLOG(INFO) << "Sub frame detected";
-        should_wrap = false;
-      }
-    }
 
-    if (should_wrap) {
-      CComObject<ProtocolSinkWrap>* wrap = NULL;
-      CComObject<ProtocolSinkWrap>::CreateInstance(&wrap);
-      DCHECK(wrap);
+  if (http_negotiate && !IsSubFrameRequest(http_negotiate)) {
+    CComObject<ProtocolSinkWrap>* wrap = NULL;
+    CComObject<ProtocolSinkWrap>::CreateInstance(&wrap);
+    DCHECK(wrap);
+    if (wrap) {
+      wrap->AddRef();
       if (wrap->Initialize(protocol, prot_sink, url)) {
         sink_to_use = wrap;
       }
+      wrap->Release();
     }
   }
 
diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc
index dd7ea8b..460f8d8 100644
--- a/chrome_frame/utils.cc
+++ b/chrome_frame/utils.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <htiframe.h>
+#include <mshtml.h>
 #include <shlobj.h>
 #include <wininet.h>
 
@@ -635,3 +637,31 @@ std::string GetRawHttpHeaders(IWinInetHttpInfo* info) {
 
   return buffer;
 }
+
+bool IsSubFrameRequest(IUnknown* service_provider) {
+  DCHECK(service_provider);
+
+  // We need to be able to get at an IWebBrowser2 if we are to decide whether
+  // this request originates from a non-top-level frame.
+  ScopedComPtr<IWebBrowser2> web_browser;
+  HRESULT hr = DoQueryService(IID_ITargetFrame2, service_provider,
+                              web_browser.Receive());
+
+  bool is_non_top_level_request = false;
+  if (web_browser) {
+    // Now check to see if we are in a sub-frame.
+    ScopedComPtr<IHTMLWindow2> current_frame, parent_frame;
+    hr = DoQueryService(IID_IHTMLWindow2, service_provider,
+                        current_frame.Receive());
+    if (current_frame) {
+      // Only the top level window will return self when get_parent is called.
+      current_frame->get_parent(parent_frame.Receive());
+      if (parent_frame != current_frame) {
+        DLOG(INFO) << "Sub frame detected";
+        is_non_top_level_request = true;
+      }
+    }
+  }
+
+  return is_non_top_level_request;
+}
diff --git a/chrome_frame/utils.h b/chrome_frame/utils.h
index 351fba1..a685691 100644
--- a/chrome_frame/utils.h
+++ b/chrome_frame/utils.h
@@ -211,6 +211,17 @@ bool IsValidUrlScheme(const std::wstring& url, bool is_privileged);
 // IWinInetHttpInfo pointer.
 std::string GetRawHttpHeaders(IWinInetHttpInfo* info);
 
+// Can be used to determine whether a given request is being performed for
+// a sub-frame or iframe in Internet Explorer. This can be called
+// from various places, notably in request callbacks and the like.
+//
+// |service_provider| must not be NULL and should be a pointer to something
+// that implements IServiceProvider (if it isn't this method returns false).
+//
+// Returns true if this method can determine with some certainty that the
+// request did NOT originate from a top level frame, returns false otherwise.
+bool IsSubFrameRequest(IUnknown* service_provider);
+
 // See COM_INTERFACE_BLIND_DELEGATE below for details.
 template <class T>
 STDMETHODIMP CheckOutgoingInterface(void* obj, REFIID iid, void** ret,