Remove race condition when installing default apps into a new profile.

chrome/browser/extensions/external_extension_provider_impl.h

Index: chrome/browser/extensions/external_extension_provider_impl.h
===================================================================
--- chrome/browser/extensions/external_extension_provider_impl.h	(revision 106274)
+++ chrome/browser/extensions/external_extension_provider_impl.h	(working copy)
@@ -50,7 +50,7 @@
 
   // Sets underlying prefs and notifies provider. Only to be called by the
   // owned ExternalExtensionLoader instance.
-  void SetPrefs(base::DictionaryValue* prefs);
+  virtual void SetPrefs(base::DictionaryValue* prefs);
 
   // ExternalExtensionProvider implementation:
   virtual void ServiceShutdown() OVERRIDE;
@@ -69,9 +69,16 @@
   static const char kExternalUpdateUrl[];
   static const char kSupportedLocales[];
 
+  // Used only for testing.
+  const std::set<std::string>& invalid_extensions() const {
+    return invalid_extensions_;
+  }
+
  protected:
   VisitorInterface* service() const { return service_; }
 
+  base::DictionaryValue* prefs() const { return prefs_.get(); }
+
  private:
   // Location for external extensions that are provided by this provider from
   // local crx files.
@@ -96,6 +103,15 @@
   // via |SetPrefs|.
   scoped_refptr<ExternalExtensionLoader> loader_;
 
+  // Extensions that were found to be invalid while processing the prefs_
+  // dictionary.  These are extensions that this provider did not even attempt
+  // to install because they failed one or more of the checks in SetPrefs().
+  //
+  // TODO(mihai): It would be nice to simply remove these extensions from
+  // prefs_, but for now, there is code that depends on these invalid extensions

[Mihai Parparita -not on Chrome, 2011/10/20 02:30:25]

I didn't mean that there was code that relies on this, just that it's a behavior
change that should be documented. Maybe add it to the CL description?

[Roger Tawa OOO till Jul 10th, 2011/10/20 14:31:15]

OK.  I think it would be better to have the comment in the code though and not
in a CL description, since this way it will be more findable by others.  You had
said:

---start quote---
The change you have to by default add extension IDs unsupported_extensions will
mean that a lot more extensions will be uninstalled by default (since if
HasExtension returns false we uninstall the extension in
ExtensionService::CheckExternalUninstall). Arguably this might be a good thing
(if the path is no longer valid because the app that installed the extension was
moved/uninstalled, we should remove the extension too), but I want us to make
that change intentionally.
---end quote---

So it seems there is code in ExtensionService that depends on this behaviour.  I
thought my comment captured your sentiment pretty well but if you'd like to
suggest different text I would be more than happy to replace my comment with
yours.

+  // still being available to HasExtension().  See how to fix this.
+  std::set<std::string> invalid_extensions_;
+
   DISALLOW_COPY_AND_ASSIGN(ExternalExtensionProviderImpl);
 };