Add a self-destruct mechanism to user-level Chrome Frame when it detects that system-level Chrome...

chrome_frame/chrome_frame_helper_util.cc

Index: chrome_frame/chrome_frame_helper_util.cc
===================================================================
--- chrome_frame/chrome_frame_helper_util.cc	(revision 85906)
+++ chrome_frame/chrome_frame_helper_util.cc	(working copy)
@@ -3,11 +3,19 @@
 // found in the LICENSE file.
 
 #include "chrome_frame/chrome_frame_helper_util.h"
+#include "chrome_tab.h"  // NOLINT
 
 #include <shlwapi.h>
+#include <stdio.h>
 
 const wchar_t kGetBrowserMessage[] = L"GetAutomationObject";

[grt (UTC plus 2), 2011/05/25 16:10:07]

nit: wrap as much of these as reasonable in namespace {}

[robertshield, 2011/05/26 15:14:28]

Done.

 
+const wchar_t kSystemLevelRegPath[] = L"Software\\Classes\\CLSID\\%s";

[grt (UTC plus 2), 2011/05/25 16:10:07]

Consider putting "Format" or "Fmt" or something in the name of this constant so
that it's clear that it's a format string.

[robertshield, 2011/05/26 15:14:28]

Done.

+const wchar_t kChromeFrameClientKey[] =

[grt (UTC plus 2), 2011/05/25 16:10:07]

Same comment about trailing separator.

[robertshield, 2011/05/26 15:14:28]

Done.

+    L"Software\\Google\\Update\\Clients\\"
+    L"{8BA986DA-5100-405E-AA35-86F34A02ACBF}\\";
+const wchar_t kChromeFrameVersionValue[] = L"pv";

[grt (UTC plus 2), 2011/05/25 16:10:07]

Hmm.  This is a Google Update value, not a Chrome Frame value.  How about
kGoogleUpdateVersionValue?

[robertshield, 2011/05/26 15:14:28]

Done.

+
 bool UtilIsWebBrowserWindow(HWND window_to_check) {
   bool is_browser_window = false;
 
@@ -185,3 +193,82 @@
   EnumChildWindows(parent, WndEnumProc, reinterpret_cast<LPARAM>(&params));
   return params.window_found_;
 }
+
+

[grt (UTC plus 2), 2011/05/25 16:10:07]

remove blank line

[robertshield, 2011/05/26 15:14:28]

Done.

+// TODO(robertshield): This is stolen shamelessly from mini_installer.cc.
+// Refactor this before (more) bad things happen.
+LONG ReadValue(HKEY key,
+               const wchar_t* value_name,
+               size_t value_size,
+               wchar_t* value) {
+  DWORD type;
+  DWORD byte_length = static_cast<DWORD>(value_size * sizeof(wchar_t));
+  LONG result = ::RegQueryValueEx(key, value_name, NULL, &type,
+                                  reinterpret_cast<BYTE*>(value),
+                                  &byte_length);
+  if (result == ERROR_SUCCESS) {
+    if (type != REG_SZ) {
+      result = ERROR_NOT_SUPPORTED;
+    } else if (byte_length == 0) {
+      *value = L'\0';
+    } else if (value[byte_length/sizeof(wchar_t) - 1] != L'\0') {
+      if ((byte_length / sizeof(wchar_t)) < value_size)
+        value[byte_length / sizeof(wchar_t)] = L'\0';
+      else
+        result = ERROR_MORE_DATA;
+    }
+  }
+  return result;
+}
+
+bool IsSystemLevelChromeFrameInstalled() {
+  wchar_t bho_clsid_as_string[MAX_PATH] = {0};
+  int count = StringFromGUID2(CLSID_ChromeFrameBHO, bho_clsid_as_string,
+                              ARRAYSIZE(bho_clsid_as_string));
+
+  bool bho_registered = false;
+  if (count > 0) {
+    wchar_t reg_path_buffer[MAX_PATH] = {0};
+    int path_count = _snwprintf(reg_path_buffer,
+                                MAX_PATH - 1,
+                                kSystemLevelRegPath,
+                                bho_clsid_as_string);
+
+    if (path_count > 0) {
+      HKEY reg_handle = NULL;
+      LONG result = RegOpenKeyEx(HKEY_LOCAL_MACHINE,
+                                 reg_path_buffer,
+                                 0,
+                                 KEY_READ,

[grt (UTC plus 2), 2011/05/25 16:10:07]

KEY_READ -> KEY_QUERY_VALUE

[robertshield, 2011/05/26 15:14:28]

Done.

+                                 &reg_handle);
+      if (result == ERROR_SUCCESS) {
+        RegCloseKey(reg_handle);
+        bho_registered = true;
+      }
+    }
+  }
+
+  // If our bho is registered, also check that we appear to have a valid

[grt (UTC plus 2), 2011/05/25 16:10:07]

Under what circumstances would the BHO be registered but CF not have a "pv"?  Is
it correct to act as if the BHO wasn't there in this case?

[robertshield, 2011/05/26 15:14:28]

As discussed, this function is now split in two parts, one which verifies the
BHO loading policy and the other that checks for a valid CF system-level
install.

+  // system-level installation.
+  bool system_level_installed = false;
+  if (bho_registered) {
+    HKEY reg_handle = NULL;
+    LONG result = RegOpenKeyEx(HKEY_LOCAL_MACHINE,
+                               kChromeFrameClientKey,
+                               0,
+                               KEY_QUERY_VALUE,
+                               &reg_handle);
+    if (result == ERROR_SUCCESS) {
+      wchar_t version_buffer[MAX_PATH] = {0};

[grt (UTC plus 2), 2011/05/25 16:10:07]

Consider giving the bho_clsid_as_string buffer a more general name and reusing
it here.

[robertshield, 2011/05/26 15:14:28]

I split the function in twine to be more specific, so can't no more.

+      result = ReadValue(reg_handle,
+                         kChromeFrameVersionValue,
+                         MAX_PATH,
+                         version_buffer);
+      if (result == ERROR_SUCCESS && wcslen(version_buffer) > 0) {

[grt (UTC plus 2), 2011/05/25 16:10:07]

wcslen(version_buffer) > 0  ->  *version_buffer != L'\0'

[robertshield, 2011/05/26 15:14:28]

Neato.

+        system_level_installed = true;
+      }

[grt (UTC plus 2), 2011/05/25 16:10:07]

add RegCloseKey(reg_handle); after this line

[robertshield, 2011/05/26 15:14:28]

Done.

+    }
+  }
+
+  return system_level_installed;
+}