Only switch to cf for text/html. With opt-in URLs we could mark a URL to be loaded in CF regardless of the target mime type. If CF turns around and wants to download the target, we would hit an infinite loop.

TEST=Verify that issue 40046 is resolved.  OptIn urls should work with the moniker patch and downloads should not cause an infinite loop.
BUG=40046

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

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

10 files changed, 242 insertions, 44 deletions

diff --git a/chrome_frame/bho.cc b/chrome_frame/bho.cc
index 7ca9bff..7a126f3 100644
--- a/chrome_frame/bho.cc
+++ b/chrome_frame/bho.cc
@@ -121,6 +121,7 @@ STDMETHODIMP Bho::BeforeNavigate2(IDispatch* dispatch, VARIANT* url,
   if (is_top_level) {
     set_url(url->bstrVal);
     set_referrer("");
+    // The moniker patch checks opt-in URLs in SniffData::DetermineRendererType.
     if (!MonikerPatchEnabled()) {
       ProcessOptInUrls(web_browser2, url->bstrVal);
     }
diff --git a/chrome_frame/http_negotiate.cc b/chrome_frame/http_negotiate.cc
index a95255a..4cc1a7d 100644
--- a/chrome_frame/http_negotiate.cc
+++ b/chrome_frame/http_negotiate.cc
@@ -28,7 +28,7 @@ const char kUACompatibleHttpHeader[] = "x-ua-compatible";
 // avoid conflict (and therefore build errors) for those building with
 // a newer Windows SDK.
 // TODO(robertshield): Remove this once we update our SDK version.
-static const int LOCAL_BINDSTATUS_SERVER_MIMETYPEAVAILABLE = 54;
+const int LOCAL_BINDSTATUS_SERVER_MIMETYPEAVAILABLE = 54;
 
 static const int kHttpNegotiateBeginningTransactionIndex = 3;
 static const int kHttpNegotiateOnResponseTransactionIndex = 4;
@@ -96,7 +96,8 @@ class SimpleBindStatusCallback : public CComObjectRootEx<CComSingleThreadModel>,
   }
 };
 
-// Attempts to get to the associated browser service for an active request.
+}  // end namespace
+
 HRESULT GetBrowserServiceFromProtocolSink(IInternetProtocolSink* sink,
                                           IBrowserService** browser_service) {
   DCHECK(browser_service);
@@ -116,8 +117,6 @@ HRESULT GetBrowserServiceFromProtocolSink(IInternetProtocolSink* sink,
   return hr;
 }
 
-}  // end namespace
-
 HttpNegotiatePatch::HttpNegotiatePatch() {
 }
 
@@ -340,6 +339,7 @@ HRESULT HttpNegotiatePatch::ReportProgress(
   if (status_code == BINDSTATUS_MIMETYPEAVAILABLE ||
       status_code == BINDSTATUS_VERIFIEDMIMETYPEAVAILABLE ||
       status_code == LOCAL_BINDSTATUS_SERVER_MIMETYPEAVAILABLE) {
+    DCHECK(lstrlenW(status_text));
     bool render_in_chrome_frame = false;
     bool is_top_level_request = !IsSubFrameRequest(me);
     // NOTE: After switching over to using the onhttpequiv notification from
@@ -395,8 +395,12 @@ HRESULT HttpNegotiatePatch::ReportProgress(
     }
 
     if (render_in_chrome_frame) {
-      DLOG(INFO) << "- changing mime type to " << kChromeMimeType;
-      status_text = kChromeMimeType;
+      if (IsTextHtmlMimeType(status_text)) {
+        DLOG(INFO) << "- changing mime type to " << kChromeMimeType;
+        status_text = kChromeMimeType;
+      } else {
+        DLOG(INFO) << "- don't want to render " << status_text << " in cf";
+      }
     }
   }
 
diff --git a/chrome_frame/http_negotiate.h b/chrome_frame/http_negotiate.h
index 3146539..6a4848b 100644
--- a/chrome_frame/http_negotiate.h
+++ b/chrome_frame/http_negotiate.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_FRAME_HTTP_NEGOTIATE_H_
 #define CHROME_FRAME_HTTP_NEGOTIATE_H_
 
+#include <shdeprecated.h>
 #include <urlmon.h>
 
 #include "base/basictypes.h"
@@ -69,4 +70,14 @@ class HttpNegotiatePatch {
   DISALLOW_COPY_AND_ASSIGN(HttpNegotiatePatch);
 };
 
+// Attempts to get to the associated browser service for an active request.
+HRESULT GetBrowserServiceFromProtocolSink(IInternetProtocolSink* sink,
+                                          IBrowserService** browser_service);
+
+// From the latest urlmon.h. Symbol name prepended with LOCAL_ to
+// avoid conflict (and therefore build errors) for those building with
+// a newer Windows SDK.
+// TODO(robertshield): Remove this once we update our SDK version.
+extern const int LOCAL_BINDSTATUS_SERVER_MIMETYPEAVAILABLE;
+
 #endif  // CHROME_FRAME_HTTP_NEGOTIATE_H_
diff --git a/chrome_frame/test/http_negotiate_unittest.cc b/chrome_frame/test/http_negotiate_unittest.cc
index a16a457..a8d2228 100644
--- a/chrome_frame/test/http_negotiate_unittest.cc
+++ b/chrome_frame/test/http_negotiate_unittest.cc
@@ -5,9 +5,12 @@
 #include <atlbase.h>
 #include <atlcom.h>
 
+#include "base/scoped_bstr_win.h"
+#include "base/scoped_comptr_win.h"
 #include "base/string_util.h"
 #include "chrome_frame/http_negotiate.h"
 #include "chrome_frame/html_utils.h"
+#include "chrome_frame/utils.h"
 #include "gtest/gtest.h"
 #include "gmock/gmock.h"
 
@@ -17,7 +20,6 @@ class HttpNegotiateTest : public testing::Test {
   }
 };
 
-
 class TestHttpNegotiate
     : public CComObjectRootEx<CComMultiThreadModel>,
       public IHttpNegotiate {
@@ -57,8 +59,9 @@ TEST_F(HttpNegotiateTest, BeginningTransaction) {
   CComObjectStackEx<TestHttpNegotiate> test_http;
   IHttpNegotiate_BeginningTransaction_Fn original =
       reinterpret_cast<IHttpNegotiate_BeginningTransaction_Fn>(
-      (*reinterpret_cast<void***>(
-        static_cast<IHttpNegotiate*>(&test_http)))[kBeginningTransactionIndex]);
+          (*reinterpret_cast<void***>(
+              static_cast<IHttpNegotiate*>(
+                  &test_http)))[kBeginningTransactionIndex]);
 
   std::wstring cf_ua(
       ASCIIToWide(http_utils::GetDefaultUserAgentHeaderWithCFTag()));
@@ -116,3 +119,152 @@ TEST_F(HttpNegotiateTest, BeginningTransaction) {
     }
   }
 }
+
+class TestInternetProtocolSink
+    : public CComObjectRootEx<CComMultiThreadModel>,
+      public IInternetProtocolSink {
+ public:
+  TestInternetProtocolSink() : status_(0) {
+    // Create an instance of IE to fullfill the requirements of being able
+    // to detect whether a sub-frame or top-frame is being loaded (see
+    // IsSubFrameRequest) and to be able to mark an IBrowserService
+    // implementation as a target for CF navigation.
+    HRESULT hr = browser_.CreateInstance(CLSID_InternetExplorer);
+    CHECK(SUCCEEDED(hr));
+    if (SUCCEEDED(hr)) {
+      browser_->Navigate(ScopedBstr(L"about:blank"), NULL, NULL, NULL, NULL);
+    }
+  }
+
+  ~TestInternetProtocolSink() {
+    if (browser_)
+      browser_->Quit();
+  }
+
+BEGIN_COM_MAP(TestInternetProtocolSink)
+  COM_INTERFACE_ENTRY(IInternetProtocolSink)
+  COM_INTERFACE_ENTRY_AGGREGATE(IID_IServiceProvider, browser_)
+END_COM_MAP()
+
+  // IInternetProtocolSink.
+  STDMETHOD(Switch)(PROTOCOLDATA* data) {
+    NOTREACHED();
+    return S_OK;
+  }
+
+  STDMETHOD(ReportProgress)(ULONG status, LPCWSTR text) {
+    status_ = status;
+    status_text_ = text ? text : L"";
+    return S_OK;
+  }
+
+  STDMETHOD(ReportData)(DWORD bscf, ULONG progress, ULONG progress_max) {
+    NOTREACHED();
+    return S_OK;
+  }
+
+  STDMETHOD(ReportResult)(HRESULT hr, DWORD err, LPCWSTR result) {
+    NOTREACHED();
+    return S_OK;
+  }
+
+  ULONG last_status() const {
+    return status_;
+  }
+
+  const std::wstring& last_status_text() const {
+    return status_text_;
+  }
+
+ protected:
+  ULONG status_;
+  std::wstring status_text_;
+  ScopedComPtr<IWebBrowser2> browser_;
+};
+
+TEST_F(HttpNegotiateTest, ReportProgress) {
+  static const int kReportProgressIndex = 4;
+  CComObjectStackEx<TestInternetProtocolSink> test_sink;
+  IInternetProtocolSink_ReportProgress_Fn original =
+      reinterpret_cast<IInternetProtocolSink_ReportProgress_Fn>(
+          (*reinterpret_cast<void***>(
+              static_cast<IInternetProtocolSink*>(
+                  &test_sink)))[kReportProgressIndex]);
+
+  struct TestCase {
+    bool mark_browser_;
+    const ULONG status_;
+    const std::wstring status_text_;
+    const std::wstring expected_status_text_;
+  } test_cases[] = {
+    // Cases where we could switch the mime type.
+    { true,
+      BINDSTATUS_MIMETYPEAVAILABLE,
+      L"text/html",
+      kChromeMimeType },
+    { false,
+      BINDSTATUS_MIMETYPEAVAILABLE,
+      L"text/html",
+      L"text/html" },
+    { true,
+      BINDSTATUS_VERIFIEDMIMETYPEAVAILABLE,
+      L"text/html",
+      kChromeMimeType },
+    { false,
+      BINDSTATUS_VERIFIEDMIMETYPEAVAILABLE,
+      L"text/html",
+      L"text/html" },
+    { true,
+      LOCAL_BINDSTATUS_SERVER_MIMETYPEAVAILABLE,
+      L"text/html",
+      kChromeMimeType },
+    { false,
+      LOCAL_BINDSTATUS_SERVER_MIMETYPEAVAILABLE,
+      L"text/html",
+      L"text/html" },
+
+    // Cases where we mark the browser but can't switch due to mime type.
+    { true,
+      BINDSTATUS_MIMETYPEAVAILABLE,
+      L"application/pdf",
+      L"application/pdf" },
+    { true,
+      BINDSTATUS_VERIFIEDMIMETYPEAVAILABLE,
+      L"application/pdf",
+      L"application/pdf" },
+    { true,
+      LOCAL_BINDSTATUS_SERVER_MIMETYPEAVAILABLE,
+      L"application/pdf",
+      L"application/pdf" },
+
+    // Cases where we should do nothing.
+    { false,
+      BINDSTATUS_CONNECTING,
+      L"foo",
+      L"foo" },
+    { false,
+      BINDSTATUS_UPLOADINGDATA,
+      L"bar",
+      L"bar" },
+  };
+
+  ScopedComPtr<IBrowserService> browser;
+  EXPECT_HRESULT_SUCCEEDED(GetBrowserServiceFromProtocolSink(&test_sink,
+      browser.Receive()));
+
+  for (int i = 0; i < arraysize(test_cases); ++i) {
+    TestCase& test = test_cases[i];
+    if (test.mark_browser_) {
+      MarkBrowserOnThreadForCFNavigation(browser);
+    }
+
+    HRESULT hr = HttpNegotiatePatch::ReportProgress(original, &test_sink,
+        test.status_, test.status_text_.c_str());
+    EXPECT_EQ(S_OK, hr);
+    // The TLS flag should always be cleared.
+    EXPECT_FALSE(CheckForCFNavigation(browser, false));
+    EXPECT_EQ(test.expected_status_text_, test_sink.last_status_text());
+    EXPECT_EQ(test.status_, test_sink.last_status());
+  }
+}
+
diff --git a/chrome_frame/test/test_mock_with_web_server.cc b/chrome_frame/test/test_mock_with_web_server.cc
index 03e0a00..71ccfb0 100644
--- a/chrome_frame/test/test_mock_with_web_server.cc
+++ b/chrome_frame/test/test_mock_with_web_server.cc
@@ -77,7 +77,7 @@ ExpectationSet MockWebBrowserEventSink::ExpectNavigationCardinality(
   } else {
     navigation += EXPECT_CALL(*this, OnBeforeNavigate2(_,
                               testing::Field(&VARIANT::bstrVal,
-                              testing::StrCaseEq(url)),_, _, _, _, _))
+                              testing::StrCaseEq(url)), _, _, _, _, _))
         .Times(cardinality);
   }
   navigation += EXPECT_CALL(*this, OnFileDownload(VARIANT_TRUE, _))
@@ -106,7 +106,7 @@ ExpectationSet MockWebBrowserEventSink::ExpectNavigation(
   ExpectationSet navigation;
   navigation += EXPECT_CALL(*this, OnBeforeNavigate2(_,
                             testing::Field(&VARIANT::bstrVal,
-                            testing::StrCaseEq(url)),_, _, _, _, _));
+                            testing::StrCaseEq(url)), _, _, _, _, _));
   navigation += EXPECT_CALL(*this, OnFileDownload(VARIANT_TRUE, _))
       .Times(testing::AnyNumber());
   navigation += EXPECT_CALL(*this, OnNavigateComplete2(_,
@@ -128,7 +128,7 @@ ExpectationSet MockWebBrowserEventSink::ExpectNavigationSequenceForAnchors(
   ExpectationSet navigation;
   navigation += EXPECT_CALL(*this, OnBeforeNavigate2(_,
                             testing::Field(&VARIANT::bstrVal,
-                            testing::StrCaseEq(url)),_, _, _, _, _))
+                            testing::StrCaseEq(url)), _, _, _, _, _))
       .Times(testing::AnyNumber());
   navigation += EXPECT_CALL(*this, OnFileDownload(VARIANT_TRUE, _))
       .Times(testing::AnyNumber());
@@ -155,7 +155,7 @@ ExpectationSet MockWebBrowserEventSink::ExpectNavigationAndSwitchSequence(
                                                           testing::Exactly(1));
   navigation += EXPECT_CALL(*this, OnBeforeNavigate2(_,
                             testing::Field(&VARIANT::bstrVal,
-                            testing::StrCaseEq(url)),_, _, _, _, _))
+                            testing::StrCaseEq(url)), _, _, _, _, _))
       .Times(testing::AnyNumber());
   navigation += EXPECT_CALL(*this, OnFileDownload(VARIANT_TRUE, _))
       .Times(testing::AnyNumber());
@@ -223,9 +223,9 @@ ACTION_P4(DelaySendScanCode, loop, delay, c, mod) {
 }
 
 ACTION_P5(SendExtendedKeysEnter, loop, delay, c, repeat, mod) {
-  const unsigned long kInterval = 25;
-  unsigned long next_delay = delay;
-  for (int i = 0; i < repeat; i++ ) {
+  const unsigned int kInterval = 25;
+  unsigned int next_delay = delay;
+  for (int i = 0; i < repeat; i++) {
     loop->PostDelayedTask(FROM_HERE, NewRunnableFunction(
         simulate_input::SendExtendedKey, c, mod), next_delay);
     next_delay += kInterval;
@@ -256,7 +256,7 @@ ACTION_P3(TypeUrlInAddressBar, loop, url, delay) {
       simulate_input::SendCharA, 'd', simulate_input::ALT),
       delay);
 
-  const unsigned long kInterval = 100;
+  const unsigned int kInterval = 100;
   int next_delay = delay + kInterval;
 
   loop->PostDelayedTask(FROM_HERE, NewRunnableFunction(
@@ -272,7 +272,7 @@ ACTION_P3(TypeUrlInAddressBar, loop, url, delay) {
 void ExpectAddressBarUrl(IWebBrowser2* web_browser2,
                          const std::wstring& expected_url) {
   EXPECT_NE(static_cast<IWebBrowser2*>(NULL), web_browser2);
-  if(web_browser2) {
+  if (web_browser2) {
     ScopedBstr address_bar_url;
     EXPECT_EQ(S_OK, web_browser2->get_LocationURL(address_bar_url.Receive()));
     EXPECT_EQ(expected_url, std::wstring(address_bar_url));
@@ -488,7 +488,7 @@ TEST_F(ChromeFrameTestWithWebServer, FLAKY_FullTabModeIE_CtrlR) {
           SetFocusToChrome(&mock),
           DelaySendChar(&loop, 1500, 'r', simulate_input::CONTROL)));
 
-  //mock.ExpectNavigation(kKeyEventUrl);
+  // mock.ExpectNavigation(kKeyEventUrl);
   EXPECT_CALL(mock, OnLoad(testing::StrCaseEq(kKeyEventUrl)))
       .WillOnce(testing::DoAll(
           VerifyAddressBarUrl(&mock),
@@ -1193,7 +1193,7 @@ TEST_F(ChromeFrameTestWithWebServer,
 
   mock.ExpectNavigationAndSwitchSequence(kSubFrameUrl2);
 
-  short bkspace = VkKeyScanA(VK_BACK);
+  short bkspace = VkKeyScanA(VK_BACK);  // NOLINT
   EXPECT_CALL(mock, OnLoad(testing::StrCaseEq(kSubFrameUrl2)))
       .WillOnce(testing::DoAll(
           SetFocusToChrome(&mock),
@@ -1322,8 +1322,10 @@ TEST_F(ChromeFrameTestWithWebServer,
 }
 
 TEST(IEPrivacy, NavigationToRestrictedSite) {
-  if (!MonikerPatchEnabled())
+  if (!MonikerPatchEnabled()) {
+    LOG(ERROR) << "Not running test. Moniker patch not enabled.";
     return;
+  }
   CloseIeAtEndOfScope last_resort_close_ie;
   chrome_frame_test::TimedMsgLoop loop;
   ComStackObjectWithUninitialize<MockWebBrowserEventSink> mock;
@@ -1375,8 +1377,10 @@ TEST(IEPrivacy, NavigationToRestrictedSite) {
 // See bug 36694 for details.  http://crbug.com/36694
 TEST_F(ChromeFrameTestWithWebServer,
        DISABLED_FullTabModeIE_TestDownloadFromForm) {
-  if (!MonikerPatchEnabled())
+  if (!MonikerPatchEnabled()) {
+    LOG(ERROR) << "Not running test. Moniker patch not enabled.";
     return;
+  }
 
   CloseIeAtEndOfScope last_resort_close_ie;
 
@@ -1415,7 +1419,7 @@ TEST_F(ChromeFrameTestWithWebServer,
   // (content-disposition is "attachment").
   class CustomResponse : public test_server::ResponseForPath {
    public:
-    CustomResponse(const char* path)
+    explicit CustomResponse(const char* path)
       : test_server::ResponseForPath(path), is_post_(false),
         post_requests_(0), get_requests_(0) {
     }
@@ -1521,8 +1525,10 @@ TEST_F(ChromeFrameTestWithWebServer,
 // to ChromeFrame correctly.
 TEST_F(ChromeFrameTestWithWebServer,
        FLAKY_FullTabModeIE_AltD_AnchorUrlNavigate) {
-  if (!MonikerPatchEnabled())
+  if (!MonikerPatchEnabled()) {
+    LOG(ERROR) << "Not running test. Moniker patch not enabled.";
     return;
+  }
 
   CloseIeAtEndOfScope last_resort_close_ie;
   chrome_frame_test::TimedMsgLoop loop;
diff --git a/chrome_frame/test/test_with_web_server.cc b/chrome_frame/test/test_with_web_server.cc
index 9e9e14f2..1687f5e 100644
--- a/chrome_frame/test/test_with_web_server.cc
+++ b/chrome_frame/test/test_with_web_server.cc
@@ -769,8 +769,10 @@ const wchar_t kAnchorUrlNavigate[] =
 // http://code.google.com/p/chromium/issues/detail?id=35341
 TEST_F(ChromeFrameTestWithWebServer,
        FLAKY_FullTabModeIE_AnchorUrlNavigateTest) {
-  if (!MonikerPatchEnabled())
+  if (!MonikerPatchEnabled()) {
+    LOG(ERROR) << "Not running test. Moniker patch not enabled.";
     return;
+  }
 
   chrome_frame_test::TimedMsgLoop loop;
   ASSERT_TRUE(LaunchBrowser(IE, kAnchorUrlNavigate));
@@ -784,8 +786,10 @@ TEST_F(ChromeFrameTestWithWebServer,
 // Test whether POST-ing a form from an mshtml page to a CF page will cause
 // the request to get reissued.  It should not.
 TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_TestPostReissue) {
-  if (!MonikerPatchEnabled())
+  if (!MonikerPatchEnabled()) {
+    LOG(ERROR) << "Not running test. Moniker patch not enabled.";
     return;
+  }
 
   chrome_frame_test::TimedMsgLoop loop;  // must come before the server.
 
@@ -818,8 +822,10 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_TestPostReissue) {
 // Test whether following a link from an mshtml page to a CF page will cause
 // multiple network requests.  It should not.
 TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_TestMultipleGet) {
-  if (!MonikerPatchEnabled())
+  if (!MonikerPatchEnabled()) {
+    LOG(ERROR) << "Not running test. Moniker patch not enabled.";
     return;
+  }
 
   chrome_frame_test::TimedMsgLoop loop;  // must come before the server.
 
diff --git a/chrome_frame/test/test_with_web_server.h b/chrome_frame/test/test_with_web_server.h
index baba20c..f1c9334 100644
--- a/chrome_frame/test/test_with_web_server.h
+++ b/chrome_frame/test/test_with_web_server.h
@@ -1,8 +1,8 @@
 // Copyright (c) 2006-2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-#ifndef CHROME_FRAME_TEST_WITH_WEB_SERVER_H_
-#define CHROME_FRAME_TEST_WITH_WEB_SERVER_H_
+#ifndef CHROME_FRAME_TEST_TEST_WITH_WEB_SERVER_H_
+#define CHROME_FRAME_TEST_TEST_WITH_WEB_SERVER_H_
 
 #include <windows.h>
 #include <string>
@@ -101,7 +101,7 @@ class ChromeFrameTestWithWebServer: public testing::Test {
 // SimpleWebServer class.
 class SimpleWebServerTest {
  public:
-  SimpleWebServerTest(int port) : server_(port), port_(port) {
+  explicit SimpleWebServerTest(int port) : server_(port), port_(port) {
   }
 
   ~SimpleWebServerTest() {
@@ -159,8 +159,5 @@ class SimpleWebServerTest {
   int port_;
 };
 
-// TODO(tommi): Remove when this is the only option.
-bool MonikerPatchEnabled();
-
-#endif  // CHROME_FRAME_TEST_WITH_WEB_SERVER_H_
+#endif  // CHROME_FRAME_TEST_TEST_WITH_WEB_SERVER_H_
 
diff --git a/chrome_frame/urlmon_bind_status_callback.cc b/chrome_frame/urlmon_bind_status_callback.cc
index 8d724f4..851edee 100644
--- a/chrome_frame/urlmon_bind_status_callback.cc
+++ b/chrome_frame/urlmon_bind_status_callback.cc
@@ -234,9 +234,9 @@ STDMETHODIMP BSCBStorageBind::OnDataAvailable(DWORD flags, DWORD size,
       PlatformThread::CurrentId());
 
   // Do not touch anything other than text/html.
-  const CLIPFORMAT text_html = RegisterClipboardFormat(CFSTR_MIME_HTML);
   bool is_interesting = (format_etc && stgmed && stgmed->pstm &&
-      (stgmed->tymed == TYMED_ISTREAM) && (text_html == format_etc->cfFormat));
+      stgmed->tymed == TYMED_ISTREAM &&
+      IsTextHtmlClipFormat(format_etc->cfFormat));
 
   if (!is_interesting) {
     // Play back report progress so far.
@@ -332,4 +332,5 @@ HRESULT BSCBStorageBind::MayPlayBack(DWORD flags) {
   }
 
   return hr;
-}
\ No newline at end of file
+}
+
diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc
index 52489a1..4642d51 100644
--- a/chrome_frame/utils.cc
+++ b/chrome_frame/utils.cc
@@ -1058,6 +1058,19 @@ int GetHttpResponseStatusFromBinding(IBinding* binding) {
   return http_status;
 }
 
+CLIPFORMAT GetTextHtmlClipboardFormat() {
+  static const CLIPFORMAT text_html = RegisterClipboardFormat(CFSTR_MIME_HTML);
+  return text_html;
+}
+
+bool IsTextHtmlMimeType(const wchar_t* mime_type) {
+  return IsTextHtmlClipFormat(RegisterClipboardFormatW(mime_type));
+}
+
+bool IsTextHtmlClipFormat(CLIPFORMAT cf) {
+  return cf == GetTextHtmlClipboardFormat();
+}
+
 ProtocolPatchMethod GetPatchMethod() {
   ProtocolPatchMethod patch_method =
       static_cast<ProtocolPatchMethod>(
@@ -1067,7 +1080,6 @@ ProtocolPatchMethod GetPatchMethod() {
 
 bool MonikerPatchEnabled() {
   ProtocolPatchMethod patch_method = GetPatchMethod();
-  LOG_IF(ERROR, patch_method != PATCH_METHOD_MONIKER)
-      << "Not running test. Moniker patch not enabled.";
   return patch_method == PATCH_METHOD_MONIKER;
 }
+
diff --git a/chrome_frame/utils.h b/chrome_frame/utils.h
index d8731ec..ab4a3cb 100644
--- a/chrome_frame/utils.h
+++ b/chrome_frame/utils.h
@@ -239,14 +239,13 @@ HRESULT DoQueryService(const IID& service_id, IUnknown* unk, T** service) {
   DCHECK(service);
   if (!unk)
     return E_INVALIDARG;
+
   ScopedComPtr<IServiceProvider> service_provider;
   HRESULT hr = service_provider.QueryFrom(unk);
-  if (!service_provider)
-    return E_NOINTERFACE;
+  if (service_provider)
+    hr = service_provider->QueryService(service_id, service);
 
-  hr = service_provider->QueryService(service_id, service);
-  if (*service == NULL)
-    return E_NOINTERFACE;
+  DCHECK(FAILED(hr) || *service);
   return hr;
 }
 
@@ -424,6 +423,15 @@ std::string GetHttpHeadersFromBinding(IBinding* binding);
 // Returns the HTTP response code from the binding passed in.
 int GetHttpResponseStatusFromBinding(IBinding* binding);
 
+// Returns the clipboard format for text/html.
+CLIPFORMAT GetTextHtmlClipboardFormat();
+
+// Returns true iff the mime type is text/html.
+bool IsTextHtmlMimeType(const wchar_t* mime_type);
+
+// Returns true iff the clipboard format is text/html.
+bool IsTextHtmlClipFormat(CLIPFORMAT cf);
+
 // Returns the desired patch method (moniker, http_equiv, protocol sink).
 // Defaults to moniker patch.
 ProtocolPatchMethod GetPatchMethod();