Make GlobalErrorService's ownership model slightly less insane.

chrome/browser/ui/global_error/global_error_service.h

Index: chrome/browser/ui/global_error/global_error_service.h
diff --git a/chrome/browser/ui/global_error/global_error_service.h b/chrome/browser/ui/global_error/global_error_service.h
index f81f7edb4da29c5b9853e258c7899a1c67e97a98..606227222016ad7225a806818f3aa473d3a481bc 100644
--- a/chrome/browser/ui/global_error/global_error_service.h
+++ b/chrome/browser/ui/global_error/global_error_service.h
@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_UI_GLOBAL_ERROR_GLOBAL_ERROR_SERVICE_H_
 #define CHROME_BROWSER_UI_GLOBAL_ERROR_GLOBAL_ERROR_SERVICE_H_
 
+#include <map>
+#include <memory>
 #include <vector>
 
 #include "base/macros.h"
@@ -21,7 +23,7 @@ class Profile;
 class GlobalErrorService : public KeyedService {
  public:
   // Type used to represent the list of currently active errors.
-  typedef std::vector<GlobalError*> GlobalErrorList;
+  using GlobalErrorList = std::vector<GlobalError*>;
 
   // Constructs a GlobalErrorService object for the given profile. The profile
   // maybe NULL for tests.
@@ -29,14 +31,20 @@ class GlobalErrorService : public KeyedService {
   ~GlobalErrorService() override;
 
   // Adds the given error to the list of global errors and displays it on
-  // browswer windows. If no browser windows are open  then the error is
-  // displayed once a browser window is opened. This class takes ownership of
-  // the error.
-  void AddGlobalError(GlobalError* error);
-
-  // Hides the given error and removes it from the list of global errors. Caller
-  // then takes ownership of the error and is responsible for deleting it.
-  void RemoveGlobalError(GlobalError* error);
+  // browser windows. If no browser windows are open then the error is
+  // displayed once a browser window is opened.
+  //
+  // The error can be added as an "owned" error, in which case the
+  // GlobalErrorService takes ownership, or as an "unowned" error in which case
+  // the error's lifetime must exceed that of the GlobalErrorService.
+  void AddOwnedGlobalError(std::unique_ptr<GlobalError> error);
+  void AddUnownedGlobalError(GlobalError* error);

[Nico, 2016/12/12 22:32:50]

Can we make this the default version (and call it AddGlobalError) and give the
Unowned version a "Do not use" comment (and file a bug for fixing this
eventually)?

[Avi (use Gerrit), 2016/12/13 03:06:19]

Done.

+
+  // Hides the given error and removes it from the list of global errors. If it
+  // was added as an owned error, ownership is returned to the caller; if it was
+  // added as an unowned error then the GlobalErrorService will not delete it.
+  std::unique_ptr<GlobalError> RemoveOwnedGlobalError(GlobalError* error);
+  void RemoveUnownedGlobalError(GlobalError* error);
 
   // Gets the error with the given command ID or NULL if no such error exists.
   // This class maintains ownership of the returned error.
@@ -51,7 +59,7 @@ class GlobalErrorService : public KeyedService {
   GlobalError* GetFirstGlobalErrorWithBubbleView() const;
 
   // Gets all errors.
-  const GlobalErrorList& errors() { return errors_; }
+  const GlobalErrorList& errors() { return all_errors_; }
 
   // Post a notification that a global error has changed and that the error UI
   // should update it self. Pass NULL for the given error to mean all error UIs
@@ -59,7 +67,8 @@ class GlobalErrorService : public KeyedService {
   void NotifyErrorsChanged(GlobalError* error);
 
  private:
-  GlobalErrorList errors_;
+  GlobalErrorList all_errors_;
+  std::map<GlobalError*, std::unique_ptr<GlobalError>> owned_errors_;
   Profile* profile_;
 
   DISALLOW_COPY_AND_ASSIGN(GlobalErrorService);