Add the Chrome version as one of the things we check to see if

we should attempt re-install. This helps robustness e.g. in the
case where we change profile directories (as we recently did).

Mark a test that I noticed was flaky (and open a bug for it).

BUG=none
TEST=none


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

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

3 files changed, 36 insertions, 8 deletions

diff --git a/ceee/ie/common/ceee_module_util.cc b/ceee/ie/common/ceee_module_util.cc
index 0b9335e..6197d30 100644
--- a/ceee/ie/common/ceee_module_util.cc
+++ b/ceee/ie/common/ceee_module_util.cc
@@ -14,6 +14,13 @@
 #include "ceee/common/process_utils_win.h"
 #include "chrome/installer/util/google_update_constants.h"
 
+#include "version.h"  // NOLINT
+
+// TODO(joi@chromium.org): would be nice to move these (and non-L counterparts)
+// to e.g. base/string_util.h
+#define LSTRINGIZE2(x) L ## #x
+#define LSTRINGIZE(x) LSTRINGIZE2(x)
+
 namespace {
 
 const wchar_t* kRegistryPath = L"SOFTWARE\\Google\\CEEE";
@@ -21,6 +28,8 @@ const wchar_t* kRegistryValueToolbandIsHidden = L"toolband_is_hidden";
 const wchar_t* kRegistryValueToolbandPlaced = L"toolband_placed";
 const wchar_t* kRegistryValueCrxInstalledPath = L"crx_installed_path";
 const wchar_t* kRegistryValueCrxInstalledTime = L"crx_installed_time";
+const wchar_t* kRegistryValueCrxInstalledByVersion =
+    L"crx_installed_runtime_version";
 
 // Global state needed by the BHO and the
 // toolband, to indicate whether ShowDW calls should affect
@@ -173,10 +182,22 @@ bool NeedToInstallExtension() {
     if (crx_path == GetInstalledExtensionPath()) {
       base::Time installed_time;
       base::PlatformFileInfo extension_info;
-      const bool success = file_util::GetFileInfo(crx_path, &extension_info);
+      bool success = file_util::GetFileInfo(crx_path, &extension_info);
       // If the call above didn't succeed, assume we need to install.
-      return !success ||
-          extension_info.last_modified > GetInstalledExtensionTime();
+      if (!success ||
+          extension_info.last_modified > GetInstalledExtensionTime()) {
+        return true;
+      } else {
+        // We also check that the current version of Chrome was the one
+        // that attempted installation; if not, changes such as a change
+        // in the location of a profile directory might have occurred, and
+        // we might need to retry installation.
+        std::wstring version_string;
+        base::win::RegKey hkcu(HKEY_CURRENT_USER, kRegistryPath, KEY_READ);
+        success = hkcu.ReadValue(
+            kRegistryValueCrxInstalledByVersion, &version_string);
+        return !success || version_string != LSTRINGIZE(CHROME_VERSION_STRING);
+      }
     }
 
     return true;
@@ -201,6 +222,9 @@ void SetInstalledExtensionPath(const FilePath& path) {
   write_result = key.WriteValue(kRegistryValueCrxInstalledPath,
                                 path.value().c_str());
   DCHECK(write_result);
+
+  write_result = key.WriteValue(kRegistryValueCrxInstalledByVersion,
+                                LSTRINGIZE(CHROME_VERSION_STRING));
 }
 
 bool IsCrxOrEmpty(const std::wstring& path) {
diff --git a/ceee/ie/common/ceee_module_util.h b/ceee/ie/common/ceee_module_util.h
index e7b2f30..1681cdb 100644
--- a/ceee/ie/common/ceee_module_util.h
+++ b/ceee/ie/common/ceee_module_util.h
@@ -51,10 +51,13 @@ std::wstring GetExtensionPath();
 FilePath GetInstalledExtensionPath();
 base::Time GetInstalledExtensionTime();
 
-// Return true if the extension is outdated compared to the registry
+// Return true if extension installation should be attempted according
+// to the registry. Note that Chrome will silently fail installation of
+// an extension version older than what is already installed.
 bool NeedToInstallExtension();
 
-// Install an extension and sets the right registry values
+// Sets the right registry values to cache the fact that we asked
+// Chrome to install an extension.
 void SetInstalledExtensionPath(const FilePath& path);
 
 // Returns true if the given path ends with .crx or is empty (as an
diff --git a/ceee/ie/common/ceee_module_util_unittest.cc b/ceee/ie/common/ceee_module_util_unittest.cc
index 67c0dd1..5e41804 100644
--- a/ceee/ie/common/ceee_module_util_unittest.cc
+++ b/ceee/ie/common/ceee_module_util_unittest.cc
@@ -22,8 +22,8 @@
 #include "ceee/testing/utils/mock_win32.h"
 #include "ceee/testing/utils/test_utils.h"
 #include "chrome/installer/util/google_update_constants.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
 
 namespace {
 
@@ -130,7 +130,8 @@ TEST_F(CeeeModuleUtilTest, ExtensionPathTestNoRegistry) {
   file_util::Delete(temp_path, true);
 }
 
-TEST_F(CeeeModuleUtilTest, ExtensionPathTest) {
+// http://code.google.com/p/chromium/issues/detail?id=62856
+TEST_F(CeeeModuleUtilTest, FLAKY_ExtensionPathTest) {
   namespace cmu = ceee_module_util;
 
   // The FilePath::Get method shouldn't be called if we take the value