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

chrome_frame/utils.cc

Index: chrome_frame/utils.cc
diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc
index ba411671abd8698ac55b642f3352feda02c2d41e..20b9b9d466931736a2a93195f7bd01b9a3fb1fbf 100644
--- a/chrome_frame/utils.cc
+++ b/chrome_frame/utils.cc
@@ -35,6 +35,7 @@
 #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"
@@ -47,25 +48,26 @@ 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 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 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 char kGCFProtocol[] = "gcf";
-const wchar_t kChromeProtocolPrefix[] = L"gcf:";
 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 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";
 
@@ -174,7 +176,7 @@ HRESULT UtilRegisterTypeLib(ITypeLib* typelib,
                                                     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")),
@@ -716,36 +718,33 @@ RendererType RendererTypeForUrl(const std::wstring& url) {
         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;
+      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;
-  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;
-  }
-
-  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;
@@ -754,6 +753,22 @@ RendererType RendererTypeForUrl(const std::wstring& 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,