Show alert failure for reloading unpacked extensions with bad manifest

chrome/browser/ui/webui/extensions/extension_loader_handler.cc

 // Copyright 2014 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/ui/webui/extensions/extension_loader_handler.h"
 
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
@@ skipping 110 matching lines @@
 void ExtensionLoaderHandler::FileHelper::FileSelected(
     const base::FilePath& path, int index, void* params) {
   loader_handler_->LoadUnpackedExtensionImpl(path);
 }
 
 void ExtensionLoaderHandler::FileHelper::MultiFilesSelected(
       const std::vector<base::FilePath>& files, void* params) {
   NOTREACHED();
 }
 
+// Struct to hold failure data when NotifyFrontendOfFailure is called before
+// the extensions page is fully loaded.
+struct ExtensionLoaderHandler::FailureData {
+  FailureData(base::FilePath file_path,

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

this should take const file path and string.

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

Done.

+              std::string error,
+              size_t line_number,
+              std::string manifest)
+      : file_path(file_path),
+        error(error),
+        line_number(line_number),
+        manifest(manifest) {}
+
+  ~FailureData() {}
+
+  base::FilePath file_path;
+  std::string error;
+  size_t line_number;
+  std::string manifest;
+};
+
+/*
+ExtensionLoaderHandler::FailureData::FailureData(base::FilePath file_path,

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

prefer this one :)

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

Done.

+                                                 std::string error,
+                                                 size_t line_number,
+                                                 std::string manifest)
+    : file_path(file_path),
+      error(error),
+      line_number(line_number),
+      manifest(manifest) {}
+*/
+
 ExtensionLoaderHandler::ExtensionLoaderHandler(Profile* profile)
     : profile_(profile),
       file_helper_(new FileHelper(this)),
+      extension_error_reporter_observer_(this),
+      display_ready_(false),
       weak_ptr_factory_(this) {
   DCHECK(profile_);
+  extension_error_reporter_observer_.Add(ExtensionErrorReporter::GetInstance());
 }
 
 ExtensionLoaderHandler::~ExtensionLoaderHandler() {
 }
 
 void ExtensionLoaderHandler::GetLocalizedValues(
     content::WebUIDataSource* source) {
   source->AddString(
       "extensionLoadErrorHeading",
       l10n_util::GetStringUTF16(IDS_EXTENSIONS_LOAD_ERROR_HEADING));
   source->AddString(
       "extensionLoadErrorMessage",
       l10n_util::GetStringUTF16(IDS_EXTENSIONS_LOAD_ERROR_MESSAGE));
   source->AddString(
       "extensionLoadErrorRetry",
       l10n_util::GetStringUTF16(IDS_EXTENSIONS_LOAD_ERROR_RETRY));
   source->AddString(
       "extensionLoadErrorGiveUp",
       l10n_util::GetStringUTF16(IDS_EXTENSIONS_LOAD_ERROR_GIVE_UP));
   source->AddString(
       "extensionLoadCouldNotLoadManifest",
       l10n_util::GetStringUTF16(IDS_EXTENSIONS_LOAD_COULD_NOT_LOAD_MANIFEST));
+  source->AddString(
+      "extensionLoadAdditionalFailures",
+      l10n_util::GetStringUTF16(IDS_EXTENSIONS_LOAD_ADDITIONAL_FAILURES));
 }
 
 void ExtensionLoaderHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
       "extensionLoaderLoadUnpacked",
       base::Bind(&ExtensionLoaderHandler::HandleLoadUnpacked,
                  weak_ptr_factory_.GetWeakPtr()));
   web_ui()->RegisterMessageCallback(
       "extensionLoaderRetry",
       base::Bind(&ExtensionLoaderHandler::HandleRetry,
                  weak_ptr_factory_.GetWeakPtr()));
+  web_ui()->RegisterMessageCallback(
+      "extensionLoaderGiveUp",
+      base::Bind(&ExtensionLoaderHandler::HandleGiveUp,

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

Even though we call it give up in the text, let's change this to what it
actually is - ignore failure.

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

Done.

+                 weak_ptr_factory_.GetWeakPtr()));
+  web_ui()->RegisterMessageCallback(
+      "extensionLoaderSetDisplayLoading",
+      base::Bind(&ExtensionLoaderHandler::HandleSetDisplayLoading,
+                 weak_ptr_factory_.GetWeakPtr()));
+  web_ui()->RegisterMessageCallback(
+      "extensionLoaderDisplayFailures",
+      base::Bind(&ExtensionLoaderHandler::HandleDisplayFailures,
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void ExtensionLoaderHandler::HandleLoadUnpacked(const base::ListValue* args) {
   DCHECK(args->empty());
   file_helper_->ChooseFile();
 }
 
 void ExtensionLoaderHandler::HandleRetry(const base::ListValue* args) {
   DCHECK(args->empty());
-  LoadUnpackedExtensionImpl(failed_path_);
+  LoadUnpackedExtensionImpl(failed_paths_.back());
+  failed_paths_.pop_back();
+}
+
+void ExtensionLoaderHandler::HandleGiveUp(const base::ListValue* args) {
+  DCHECK(args->empty());
+  failed_paths_.pop_back();
+}
+
+void ExtensionLoaderHandler::HandleSetDisplayLoading(
+    const base::ListValue* args) {
+  DCHECK(args->empty());
+  display_ready_ = false;
+}
+
+void ExtensionLoaderHandler::HandleDisplayFailures(
+    const base::ListValue* args) {
+  DCHECK(args->empty());
+  display_ready_ = true;
+
+  if (failures_.empty())
+    return;
+
+  for (size_t i = 0; i < failures_.size(); ++i) {
+    FailureData* failure = failures_[i];
+    NotifyFrontendOfFailure(failure->file_path,

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

I'd still like to be able to notify the frontend in one big batch, and have it
sort it out.  Doing it this way could result in a flicker.

+                            failure->error,
+                            failure->line_number,
+                            failure->manifest);
+  }
+  failures_.clear();
 }
 
 void ExtensionLoaderHandler::LoadUnpackedExtensionImpl(
     const base::FilePath& file_path) {
   scoped_refptr<UnpackedInstaller> installer = UnpackedInstaller::Create(
       ExtensionSystem::Get(profile_)->extension_service());
-  installer->set_on_failure_callback(
-      base::Bind(&ExtensionLoaderHandler::OnLoadFailure,
-                 weak_ptr_factory_.GetWeakPtr()));
 
   // We do our own error handling, so we don't want a load failure to trigger
   // a dialog.
   installer->set_be_noisy_on_failure(false);
 
   installer->Load(file_path);
 }
 
 void ExtensionLoaderHandler::OnLoadFailure(const base::FilePath& file_path,
                                            const std::string& error) {
-  failed_path_ = file_path;
+  failed_paths_.push_back(file_path);
   size_t line = 0u;
   size_t column = 0u;
   std::string regex =
       base::StringPrintf("%s  Line: (\\d+), column: (\\d+), Syntax error.",
                          manifest_errors::kManifestParseError);
   // If this was a JSON parse error, we can highlight the exact line with the
   // error. Otherwise, we should still display the manifest (for consistency,
   // reference, and so that if we ever make this really fancy and add an editor,
   // it's ready).
   //
@@ skipping 11 matching lines @@
                  file_path,
                  error,
                  line));
 }
 
 void ExtensionLoaderHandler::NotifyFrontendOfFailure(
     const base::FilePath& file_path,
     const std::string& error,
     size_t line_number,
     const std::string& manifest) {
+  // If the extensions page is not loaded, delay frontend notification.
+  if (!display_ready_) {
+    failures_.push_back(new FailureData(file_path,
+                                        error,
+                                        line_number,
+                                        manifest));
+    return;
+  }
+
   base::StringValue file_value(file_path.LossyDisplayName());
   base::StringValue error_value(base::UTF8ToUTF16(error));
 
   base::DictionaryValue manifest_value;
   SourceHighlighter highlighter(manifest, line_number);
   // If the line number is 0, this highlights no regions, but still adds the
   // full manifest.
   highlighter.SetHighlightedRegions(&manifest_value);
 
   web_ui()->CallJavascriptFunction(
       "extensions.ExtensionLoader.notifyLoadFailed",
       file_value,
       error_value,
       manifest_value);
 }
 
 }  // namespace extensions