Introduce IEImporterTestRegistryOverrider to be able to override the registry for a test and flag t…

chrome/browser/importer/ie_importer.cc

Index: chrome/browser/importer/ie_importer.cc
diff --git a/chrome/browser/importer/ie_importer.cc b/chrome/browser/importer/ie_importer.cc
index 6f055a53f4de0d151de05960a93e998c159bd5a5..bfcfd0164ddd9d344c3633e71059addf40c761a4 100644
--- a/chrome/browser/importer/ie_importer.cc
+++ b/chrome/browser/importer/ie_importer.cc
@@ -419,6 +419,14 @@ void IEImporter::StartImport(const importer::SourceProfile& source_profile,
   bridge_ = bridge;
   source_path_ = source_profile.source_path;
 
+  // If there is indication that an override is required, but we fail to set it,
+  // prefer returning early to running the test with whatever is in the real
+  // registry.
+  if (!test_registry_overrider_.StartRegistryOverrideIfNeeded()) {

[robertshield, 2013/04/26 19:20:53]

This doesn't disambiguate between the case where there is an unspecified error
reading the environment var vs. the case where the env var is successfully read,
but HKCU could not be overridden. 

In the latter case, it makes sense to abort, not sure that it makes sense to
abort in the former case too.

[gab, 2013/04/29 02:00:43]

Agreed, changed the method to always return true on failed read, I wanted to
DCHECK that the failure was ERROR_ENVVAR_NOT_FOUND, but it doesn't seem
base::Environment does it, nor do I like relying on how it uses
::GetEnvironmentVariable() to call ::GetLastError() in my code if GetVar()
returns false.

FWIW, I find base::Environment is a bit weak-sauce in its error-checking, but
I'm not going to change that :).

+    NOTREACHED();
+    return;
+  }
+
   bridge_->NotifyStarted();
 
   if ((items & importer::HOME_PAGE) && !cancelled())