Show alert failure for reloading unpacked extensions with bad manifest

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

Index: chrome/browser/ui/webui/extensions/extension_loader_handler.h
diff --git a/chrome/browser/ui/webui/extensions/extension_loader_handler.h b/chrome/browser/ui/webui/extensions/extension_loader_handler.h
index 5d0351f97a0bfb200b2b64473ed49437a482393f..aaffe362c0e391f2611980d29b578e4cee3fb133 100644
--- a/chrome/browser/ui/webui/extensions/extension_loader_handler.h
+++ b/chrome/browser/ui/webui/extensions/extension_loader_handler.h
@@ -6,18 +6,20 @@
 #define CHROME_BROWSER_UI_WEBUI_EXTENSIONS_EXTENSION_LOADER_HANDLER_H_
 
 #include <string>
+#include <vector>
 
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "base/scoped_observer.h"
+#include "base/values.h"
+#include "chrome/browser/extensions/extension_error_reporter.h"
+#include "content/public/browser/navigation_controller.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_ui_message_handler.h"
 
-namespace base {
-class ListValue;
-}
-
 namespace content {
 class WebUIDataSource;
 }
@@ -29,7 +31,9 @@ namespace extensions {
 class Extension;
 
 // The handler page for the Extension Commands UI overlay.
-class ExtensionLoaderHandler : public content::WebUIMessageHandler {
+class ExtensionLoaderHandler : public content::WebUIMessageHandler,
+                               public ExtensionErrorReporter::Observer,
+                               public content::WebContentsObserver {
  public:
   explicit ExtensionLoaderHandler(Profile* profile);
   virtual ~ExtensionLoaderHandler();
@@ -49,19 +53,37 @@ class ExtensionLoaderHandler : public content::WebUIMessageHandler {
   // Handle the 'extensionLoaderRetry' message.
   void HandleRetry(const base::ListValue* args);
 
+  // Handle the 'extensionLoaderIgnoreFailure' message.
+  void HandleIgnoreFailure(const base::ListValue* args);
+
+  // Handle the 'extensionLoaderSetDisplayLoading' message.
+  void HandleSetDisplayLoading(const base::ListValue* args);
+
+  // Handle the 'extensionLoaderDisplayFailures' message.
+  void HandleDisplayFailures(const base::ListValue* args);
+
   // Try to load an unpacked extension from the given |file_path|.
   void LoadUnpackedExtensionImpl(const base::FilePath& file_path);
 
-  // Called when an unpacked extension fails to load.
-  void OnLoadFailure(const base::FilePath& file_path, const std::string& error);
+  // ExtensionErrorReporter::Observer implementation.

[Finnur, 2014/07/14 10:25:58]

Prefer shorter version:
// ExtensionErrorReporter::Observer:
... over ...
// ExtensionErrorReporter::Observer implementation.

Same with line 72.

[gpdavis, 2014/07/14 18:58:20]

Done.

+  virtual void OnLoadFailure(const base::FilePath& file_path,
+                             const std::string& error) OVERRIDE;
 
-  // Notify the frontend of the failure. If it was a manifest error, |manifest|
-  // will hold the manifest contents, and |line_number| will point to the line
-  // at which the error was found.
-  void NotifyFrontendOfFailure(const base::FilePath& file_path,
-                               const std::string& error,
-                               size_t line_number,
-                               const std::string& manifest);
+  // content::WebContentsObserver implementation.
+  virtual void DidStartNavigationToPendingEntry(
+      const GURL& url,
+      content::NavigationController::ReloadType reload_type) OVERRIDE;
+
+  // Add a failure to failures_. If it was a manifest error, |manifest| will
+  // hold the manifest contents, and |line_number| will point to the line at
+  // which the error was found.
+  void AddFailure(const base::FilePath& file_path,
+                  const std::string& error,
+                  size_t line_number,
+                  const std::string& manifest);
+
+  // Notify the frontend of all failures.
+  void NotifyFrontendOfFailure();
 
   // The profile with which this Handler is associated.
   Profile* profile_;
@@ -69,9 +91,19 @@ class ExtensionLoaderHandler : public content::WebUIMessageHandler {
   // A helper to manage file picking.
   scoped_ptr<FileHelper> file_helper_;
 
-  // The file path to the extension that failed to load, or empty. This is
-  // loaded when the user selects "retry".
-  base::FilePath failed_path_;
+  // Holds information about all unpacked extension install failures that
+  // were reported while the extensions page was loading.
+  base::ListValue failures_;
+
+  // Holds failed paths for load retries.
+  std::vector<base::FilePath> failed_paths_;
+
+  ScopedObserver<ExtensionErrorReporter, ExtensionErrorReporter::Observer>
+      extension_error_reporter_observer_;
+
+  // Set when the chrome://extensions page is fully loaded and the load
+  // failures UI is ready to receive failures.

[Finnur, 2014/07/14 10:25:58]

s/the load failures UI/the UI showing load failures/
s/receive failures/show failure notifications/ ?

It is also not quite clear to me why you need this variable. Can you document it
a bit?

[gpdavis, 2014/07/14 18:58:19]

Done.

+  bool display_ready_;

[Finnur, 2014/07/14 10:25:58]

This variable is a bit general (e.g. Display can mean Monitor). Can you find a
better name?

[gpdavis, 2014/07/14 18:58:20]

Understood.  How does ui_ready_ sound?  That seems less ambiguous to  me.

[Finnur, 2014/07/15 10:38:45]

Sure.

 
   // Weak pointer factory for posting background tasks.
   base::WeakPtrFactory<ExtensionLoaderHandler> weak_ptr_factory_;