Add a setting to CF to remove 'chromeframe' from the UserAgent on a per-pattern basis.

chrome_frame/utils.cc

 // Copyright (c) 2012 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.
 
 #include "chrome_frame/utils.h"
 
 #include <atlsafe.h>
 #include <atlsecurity.h>
 #include <htiframe.h>
 #include <mshtml.h>
@@ skipping 17 matching lines @@
 #include "base/win/scoped_variant.h"
 #include "chrome/common/automation_messages.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/installer/util/chrome_frame_distribution.h"
 #include "chrome_frame/chrome_tab.h"
 #include "chrome_frame/extra_system_apis.h"
 #include "chrome_frame/html_utils.h"
 #include "chrome_frame/navigation_constraints.h"
 #include "chrome_frame/policy_settings.h"
+#include "chrome_frame/registry_list_preferences_holder.h"
 #include "chrome_frame/simple_resource_loader.h"
 #include "googleurl/src/gurl.h"
 #include "googleurl/src/url_canon.h"
 #include "grit/chromium_strings.h"
 #include "net/base/escape.h"
 #include "net/http/http_util.h"
 #include "ui/base/models/menu_model.h"
 
 using base::win::RegKey;
 
 // Note that these values are all lower case and are compared to

[grt (UTC plus 2), 2012/03/20 17:27:45]

from the "no good deed goes unpunished" department: would you update this
comment?  not all values below are lower case, so it's not clear to me what
"these values" corresponds to.  since some of these constants are the names of
registry values, perhaps make those ones kRegShadyGroove to group them together.

[robertshield, 2012/03/26 02:43:33]

Reordered and updated the comment.

 // lower-case-transformed values.
+const char kGCFProtocol[] = "gcf";
+
+const wchar_t kAllowUnsafeURLs[] = L"AllowUnsafeURLs";
+const wchar_t kBodyTag[] = L"body";
+const wchar_t kChromeFrameConfigKey[] = L"Software\\Google\\ChromeFrame";
+const wchar_t kContentAttribName[] = L"content";
+const wchar_t kChromeContentPrefix[] = L"chrome=";
+const wchar_t kChromeMimeType[] = L"application/chromepage";
+const wchar_t kChromeProtocolPrefix[] = L"gcf:";
+const wchar_t kEnableBuggyBhoIntercept[] = L"EnableBuggyBhoIntercept";
+const wchar_t kEnableGCFRendererByDefault[] = L"IsDefaultRenderer";
+const wchar_t kExcludeUAFromDomainList[] = L"ExcludeUAFromDomain";
+const wchar_t kHttpEquivAttribName[] = L"http-equiv";
+const wchar_t kIexploreProfileName[] = L"iexplore";
 const wchar_t kMetaTag[] = L"meta";
-const wchar_t kHttpEquivAttribName[] = L"http-equiv";
-const wchar_t kContentAttribName[] = L"content";
-const wchar_t kXUACompatValue[] = L"x-ua-compatible";
-const wchar_t kBodyTag[] = L"body";
-const wchar_t kChromeContentPrefix[] = L"chrome=";
-const char kGCFProtocol[] = "gcf";
-const wchar_t kChromeProtocolPrefix[] = L"gcf:";
-const wchar_t kChromeMimeType[] = L"application/chromepage";
 const wchar_t kPatchProtocols[] = L"PatchProtocols";
-const wchar_t kChromeFrameConfigKey[] = L"Software\\Google\\ChromeFrame";
 const wchar_t kRenderInGCFUrlList[] = L"RenderInGcfUrls";
 const wchar_t kRenderInHostUrlList[] = L"RenderInHostUrls";
-const wchar_t kEnableGCFRendererByDefault[] = L"IsDefaultRenderer";
-const wchar_t kIexploreProfileName[] = L"iexplore";
 const wchar_t kRundllProfileName[] = L"rundll32";
-
-const wchar_t kAllowUnsafeURLs[] = L"AllowUnsafeURLs";
-const wchar_t kEnableBuggyBhoIntercept[] = L"EnableBuggyBhoIntercept";
+const wchar_t kXUACompatValue[] = L"x-ua-compatible";
 
 static const wchar_t kChromeFramePersistNPAPIReg[] = L"PersistNPAPIReg";
 
 const char kAttachExternalTabPrefix[] = "attach_external_tab";
 
 // Indicates that we are running in a test environment, where execptions, etc
 // are handled by the chrome test crash server.
 const wchar_t kChromeFrameHeadlessMode[] = L"ChromeFrameHeadlessMode";
 
 // Indicates that we are running in an environment that expects chrome renderer
@@ skipping 88 matching lines @@
 }
 
 HRESULT UtilRegisterTypeLib(ITypeLib* typelib,
                             LPCWSTR typelib_path,
                             LPCWSTR help_dir,
                             bool for_current_user_only) {
   typedef HRESULT(WINAPI *RegisterTypeLibPrototype)(ITypeLib FAR* type_lib,
                                                     OLECHAR FAR* full_path,
                                                     OLECHAR FAR* help_dir);
   LPCSTR function_name =
-    for_current_user_only ? "RegisterTypeLibForUser" : "RegisterTypeLib";
+      for_current_user_only ? "RegisterTypeLibForUser" : "RegisterTypeLib";
   RegisterTypeLibPrototype reg_tlb =
       reinterpret_cast<RegisterTypeLibPrototype>(
           GetProcAddress(GetModuleHandle(_T("oleaut32.dll")),
                                          function_name));
   if (NULL == reg_tlb) {
     return E_FAIL;
   }
   return reg_tlb(typelib,
                  const_cast<OLECHAR*>(typelib_path),
                  const_cast<OLECHAR*>(help_dir));
@@ skipping 521 matching lines @@
       PolicySettings::GetInstance()->GetRendererForUrl(url.c_str());
   if (renderer != PolicySettings::RENDERER_NOT_SPECIFIED) {
     // We may know at this point that policy says do NOT render in Chrome Frame.
     // To maintain consistency, we return RENDERER_TYPE_UNDETERMINED so that
     // content sniffing, etc. still take place.
     // TODO(tommi): Clarify the intent here.
     return (renderer == PolicySettings::RENDER_IN_CHROME_FRAME) ?
         RENDERER_TYPE_CHROME_OPT_IN_URL : RENDERER_TYPE_UNDETERMINED;
   }
 
-  RegKey config_key;
-  if (config_key.Open(HKEY_CURRENT_USER, kChromeFrameConfigKey,
-                      KEY_READ) != ERROR_SUCCESS) {
-    return RENDERER_TYPE_UNDETERMINED;
-  }
+  static int render_in_cf_by_default = FALSE;
+  static RegistryListPreferencesHolder render_type_for_url_holder;

[grt (UTC plus 2), 2012/03/20 17:27:45]

this introduces an exit-time call to a dtor.  if leaking this object is okay
(we're pinned anway), please use:
CR_DEFINE_STATIC_LOCAL(RegistryListPreferencesHolder,
render_type_for_url_holder, ());
also note that this isn't threadsafe.  prefer
LazyInstance<RegistryListPreferencesHolder>::Leaky if this could be called from
multiple threads.

[robertshield, 2012/03/26 02:43:33]

Done.

 
   RendererType renderer_type = RENDERER_TYPE_UNDETERMINED;
+  if (!render_type_for_url_holder.Valid()) {

[grt (UTC plus 2), 2012/03/20 17:27:45]

i think this is equivalent, but smaller:
  if (!render_type_for_url_holder.Valid()) {
    const wchar_t* url_list_name = kRenderInGCFUrlList;
    render_in_cf_by_default = GetConfigInt(FALSE, kEnableGCFRendererByDefault);
    if (render_in_cf_by_default) {
      url_list_name = kRenderInHostUrlList;
      renderer_type = RENDERER_TYPE_CHROME_DEFAULT_RENDERER;
    }
    render_type_for_url_holder.Init(HKEY_CURRENT_USER, kChromeFrameConfigKey,
                                    url_list_name);
  }

[robertshield, 2012/03/26 02:43:33]

Done.

+    RegKey config_key;
+    if (config_key.Open(HKEY_CURRENT_USER, kChromeFrameConfigKey,
+                        KEY_READ) == ERROR_SUCCESS) {
+      config_key.ReadValueDW(
+          kEnableGCFRendererByDefault,
+          reinterpret_cast<DWORD*>(&render_in_cf_by_default));
 
-  const wchar_t* url_list_name = NULL;
-  int render_in_cf_by_default = FALSE;
-  config_key.ReadValueDW(kEnableGCFRendererByDefault,
-                         reinterpret_cast<DWORD*>(&render_in_cf_by_default));
-  if (render_in_cf_by_default) {
-    url_list_name = kRenderInHostUrlList;
-    renderer_type = RENDERER_TYPE_CHROME_DEFAULT_RENDERER;
-  } else {
-    url_list_name = kRenderInGCFUrlList;
-  }
+      const wchar_t* url_list_name = NULL;
+      if (render_in_cf_by_default) {
+        url_list_name = kRenderInHostUrlList;
+        renderer_type = RENDERER_TYPE_CHROME_DEFAULT_RENDERER;
+      } else {
+        url_list_name = kRenderInGCFUrlList;
+      }
 
-  bool match_found = false;
-  base::win::RegistryValueIterator url_list(config_key.Handle(), url_list_name);
-  while (!match_found && url_list.Valid()) {
-    if (MatchPattern(url, url_list.Name())) {
-      match_found = true;
-    } else {
-      ++url_list;
+      render_type_for_url_holder.Init(HKEY_CURRENT_USER,
+                                      kChromeFrameConfigKey,
+                                      url_list_name);
     }
   }
 
-  if (match_found) {
+  if (render_type_for_url_holder.ListMatches(url)) {
     renderer_type = render_in_cf_by_default ?
       RENDERER_TYPE_UNDETERMINED :
       RENDERER_TYPE_CHROME_OPT_IN_URL;
   }
 
   return renderer_type;
 }
 
+UserAgentAction ShouldRemoveUAForUrl(const string16& url) {
+  // TODO(robertshield): Wire up the stuff in PolicySettings here so the value
+  // can be specified via group policy.
+  static RegistryListPreferencesHolder user_agent_filter_holder;

[grt (UTC plus 2), 2012/03/20 17:27:45]

see note above about static dtors and threadsafety

[robertshield, 2012/03/26 02:43:33]

Done.

+  if (!user_agent_filter_holder.Valid()) {
+    user_agent_filter_holder.Init(HKEY_CURRENT_USER,
+                                  kChromeFrameConfigKey,
+                                  kExcludeUAFromDomainList);

[slightlyoff1, 2012/03/20 15:31:56]

My biggest concern here is that we'll provide some values to this as a form of
compatibility list and then users will augment the list and on the next upgrade
we'll be owned. Can we have our own (ostensibly) private key where we look for
default values and then check this location for user-provided additions to the
list?

[robertshield, 2012/03/26 02:43:33]

That sounds reasonable, but I'd like to tackle that in a different CL if that's
okay with you. This CL doesn't provide for a default list and several other
changes will also be needed to make that happen; I'd rather make the change you
suggest then. I've left a TODO in place.

+  }
+
+  if (user_agent_filter_holder.ListMatches(url)) {

[grt (UTC plus 2), 2012/03/20 17:27:45]

nit: remove braces

[robertshield, 2012/03/26 02:43:33]

Done.

+    return USER_AGENT_REMOVE;
+  }
+  return USER_AGENT_DEFAULT;
+}
+
 HRESULT NavigateBrowserToMoniker(IUnknown* browser, IMoniker* moniker,
                                  const wchar_t* headers, IBindCtx* bind_ctx,
                                  const wchar_t* fragment, IStream* post_data,
                                  VARIANT* flags) {
   DCHECK(browser);
   DCHECK(moniker);
   DCHECK(bind_ctx);
 
   base::win::ScopedComPtr<IWebBrowser2> web_browser2;
   HRESULT hr = DoQueryService(SID_SWebBrowserApp, browser,
@@ skipping 865 matching lines @@
 
     ret = InternetSetOption(NULL, connection_options[option_index],
                             &connections, connection_value_size);
     if (!ret) {
       return false;
     }
   }
   wininet_connection_count_updated = true;
   return true;
 }