Show alert failure for reloading unpacked extensions with bad manifest

chrome/browser/background/background_contents_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/background/background_contents_service.h"
 
 #include "apps/app_load_service.h"
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 111 matching lines @@
       if (!service->GetAppBackgroundContents(
               base::ASCIIToUTF16(copied_extension_id))) {
         service->LoadBackgroundContentsForExtension(profile_,
                                                     copied_extension_id);
       }
     } else if (is_platform_app_) {
       apps::AppLoadService::Get(profile_)->
           RestartApplication(copied_extension_id);
     } else {
       extensions::ExtensionSystem::Get(profile_)->extension_service()->
-          ReloadExtension(copied_extension_id);
+          ReloadExtension(copied_extension_id, true);

[Devlin, 2014/07/02 17:46:50]

all of these should have anonymous bool comments...

On second thought.... I'm beginning to doubt whether or not we should force
every caller to do this.  It creates a bit more noise in the CL than I was
expecting.  Let's go back to making two functions,
ReloadExtension() and ReloadExtensionWithQuietFailure().
Sorry for the extra work - I should have looked at all the call sites before I
requested it :/

[gpdavis, 2014/07/02 19:13:54]

Not a problem!  This is why I suggested overloading the method.

The only problem here is that ReloadExtension needs to have the boolean
argument, since it creates and uses the unpacked installer.  The other option
would be having a flag in the class that's used in that method, and another
method to set that flag.  That seems messy, but I couldn't simply add a
ReloadExtensionWithQuietFailure without either copying the method with a minor
change, or adding the boolean flag.  That's why I suggested having one that
takes the boolean flag, and overloading it with one that doesn't that can call
the other with true by default.

I'll take a look at it and see if I can come up with any other solutions.

[Devlin, 2014/07/02 19:16:57]

Our usual tactic for this is something like:
ExtensionService {
 public:
  ReloadExtension() {
    ReloadExtensionImpl(true /* be noisy */);
  }
  ReloadExtensionWithQuietFailure() {
    ReloadExtensionImpl(false /* don't be noisy */);
  }
 private:
  ReloadExtensionImpl(bool be_noisy) {
    ...
  }
}

It keeps the public interface fairly clean, and makes it clear what's happening
in all cases.

     }
 
     // Closing the crash notification balloon for the app/extension here should
     // be OK, but it causes a crash on Mac, see: http://crbug.com/78167
     ScheduleCloseBalloon(copied_extension_id);
   }
 
   virtual bool HasClickedListener() OVERRIDE { return true; }
 
   virtual std::string id() const OVERRIDE {
@@ skipping 87 matching lines @@
       !extension_registry) {
     return;
   }
 
   if (!extension_registry->GetExtensionById(
           extension_id, extensions::ExtensionRegistry::TERMINATED)) {
     // Either the app/extension was uninstalled by policy or it has since
     // been restarted successfully by someone else (the user).
     return;
   }
-  extension_system->extension_service()->ReloadExtension(extension_id);
+  extension_system->extension_service()->ReloadExtension(extension_id, true);
 }
 
 }  // namespace
 
 // Keys for the information we store about individual BackgroundContents in
 // prefs. There is one top-level DictionaryValue (stored at
 // prefs::kRegisteredBackgroundContents). Information about each
 // BackgroundContents is stored under that top-level DictionaryValue, keyed
 // by the parent application ID for easy lookup.
 //
@@ skipping 535 matching lines @@
     bool user_gesture,
     bool* was_blocked) {
   Browser* browser = chrome::FindLastActiveWithProfile(
       Profile::FromBrowserContext(new_contents->GetBrowserContext()),
       chrome::GetActiveDesktop());
   if (browser) {
     chrome::AddWebContents(browser, NULL, new_contents, disposition,
                            initial_pos, user_gesture, was_blocked);
   }
 }