Change ChooserController ownership model

chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.h

Index: chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.h
diff --git a/chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.h b/chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.h
index d5c9093c89369f37ad3d84de0ace426c9dea28c8..11d94637c4574c420441dfab29a74edc551a47d6 100644
--- a/chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.h
+++ b/chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.h
@@ -7,21 +7,23 @@
 
 #import <Cocoa/Cocoa.h>
 
+#include <memory>
+
 #include "base/macros.h"
 #include "components/bubble/bubble_ui.h"
-#include "components/chooser_controller/chooser_controller.h"
 
 class Browser;
 @class ChooserBubbleUiController;
+class ChooserController;
 
 // ChooserBubbleUiCocoa implements a chooser-based permission model.
 // It uses |NSTableView| to show a list of options for user to grant
 // permission. It can be used by the WebUSB or WebBluetooth APIs.
 // It is owned by the BubbleController, which is owned by the BubbleManager.
-class ChooserBubbleUiCocoa : public BubbleUi,
-                             public ChooserController::Observer {
+class ChooserBubbleUiCocoa : public BubbleUi {
  public:
-  ChooserBubbleUiCocoa(Browser* browser, ChooserController* chooser_controller);
+  ChooserBubbleUiCocoa(Browser* browser,
+                       std::unique_ptr<ChooserController> chooser_controller);
   ~ChooserBubbleUiCocoa() override;
 
   // BubbleUi:
@@ -29,17 +31,17 @@ class ChooserBubbleUiCocoa : public BubbleUi,
   void Close() override;
   void UpdateAnchorPosition() override;
 
-  // ChooserController::Observer:
-  void OnOptionsInitialized() override;
-  void OnOptionAdded(size_t index) override;
-  void OnOptionRemoved(size_t index) override;
-
   // Called when |chooser_bubble_ui_controller_| is closing.
   void OnBubbleClosing();
 
  private:
-  Browser* browser_;                       // Weak.
-  ChooserController* chooser_controller_;  // Weak.
+  Browser* browser_;  // Weak.
+  // |chooser_controller_| is not owned by this class, it is owned by

[Robert Sesek, 2016/06/22 21:46:30]

This seems significantly more confusing than the current ownership model. Views
generally don't own things in Cocoa; their controllers do.

[juncai, 2016/06/22 22:41:21]

The current ownership model causes many places to have a weak pointer of
ChooserController* chooser_controller_, and it makes safely using that weak
pointer hard. By changing it to let ChooserContentView[Cocoa] own the
ChooserController, it removes almost all of the ChooserController weak pointers
except at ChromeExtensionBluetoothChooser and BluetoothChooserDesktop classes.
And in that case, I think it is safe to store and use it (explained at another
comment).

Also ChooserContentView[Cocoa] is used by both the chooser bubble and dialog. If
let their controllers own |chooser_controller_|, the two controllers will have
some similar redundant code that uses the |chooser_controller_|.

+  // ChooserContentViewCocoa.
+  // This field only temporarily owns the ChooserController. It is moved
+  // into the ChooserContentViewCocoa when BuildBubbleUi() is called and
+  // the bubble is shown.
+  std::unique_ptr<ChooserController> chooser_controller_;
   // Cocoa-side chooser bubble UI controller. Weak, as it will close itself.
   ChooserBubbleUiController* chooser_bubble_ui_controller_;