new implementation of default apps

chrome/browser/extensions/extension_service.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/extension_service.h"
 
 #include <algorithm>
 #include <set>
 
 #include "base/basictypes.h"
@@ skipping 2243 matching lines @@
          const Version* version,
          const FilePath& path,
          Extension::Location location,
          int creation_flags,
          bool mark_acknowledged) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   CHECK(Extension::IdIsValid(id));
   if (extension_prefs_->IsExternalExtensionUninstalled(id))
     return false;
 
-  DCHECK(version);
-
   // Before even bothering to unpack, check and see if we already have this
   // version. This is important because these extensions are going to get
   // installed on every startup.
   const Extension* existing = GetExtensionById(id, true);
-  if (existing) {
+
+  // The default apps will have the location set as INTERNAL. Since older
+  // default apps are installed as EXTERNAL, we override them. However, if the
+  // app is already installed as internal, then do the version check.
+  if (existing && (location != Extension::INTERNAL ||

[Mihai Parparita -not on Chrome, 2012/08/15 00:32:13]

I found this boolean expression hard to read because of all the negations. Can
you instead have a helper variable like:
bool is_default_apps_migration = location == Extension::INTERNAL &&
Extension::IsExternalLocation(existing->location());
if (existing && !is_default_apps_migration) {
  ...

+      !Extension::IsExternalLocation(existing->location()))) {
+
+    DCHECK(version);
+
     switch (existing->version()->CompareTo(*version)) {
       case -1:  // existing version is older, we should upgrade
         break;
       case 0:  // existing version is same, do nothing
         return false;
       case 1:  // existing version is newer, uh-oh
         LOG(WARNING) << "Found external version of extension " << id
                      << "that is older than current version. Current version "
-                     << "is: " << existing->VersionString() << ". New version "
-                     << "is: " << version->GetString()
+                     << "is: " << existing->VersionString() << ". New "
+                     << "version is: " << version->GetString()
                      << ". Keeping current version.";
         return false;
     }
   }
 
   // If the extension is already pending, don't start an install.
   if (!pending_extension_manager()->AddFromExternalFile(
           id, location, *version)) {
     return false;
   }
@@ skipping 261 matching lines @@
 
 ExtensionService::NaClModuleInfoList::iterator
     ExtensionService::FindNaClModule(const GURL& url) {
   for (NaClModuleInfoList::iterator iter = nacl_module_list_.begin();
        iter != nacl_module_list_.end(); ++iter) {
     if (iter->url == url)
       return iter;
   }
   return nacl_module_list_.end();
 }