Mac Web Intents Part 15: Inline extension install prompt (model)

chrome/browser/extensions/extension_install_dialog.cc

Index: chrome/browser/extensions/extension_install_dialog.cc
diff --git a/chrome/browser/extensions/extension_install_dialog.cc b/chrome/browser/extensions/extension_install_dialog.cc
index a3cd87bf911ec99916e8988d0b5bd51e1665fcee..00d7fec4602c8588f540822a55ee033cbb9988d2 100644
--- a/chrome/browser/extensions/extension_install_dialog.cc
+++ b/chrome/browser/extensions/extension_install_dialog.cc
@@ -59,14 +59,17 @@ AutoConfirmForTest CheckAutoConfirmCommandLineSwitch() {
 
 }  // namespace
 
-void ShowExtensionInstallDialog(gfx::NativeWindow parent,
-                                content::PageNavigator* navigator,
-                                ExtensionInstallPrompt::Delegate* delegate,
-                                const ExtensionInstallPrompt::Prompt& prompt) {
+void ShowExtensionInstallDialog(
+    gfx::NativeWindow parent,
+    content::PageNavigator* navigator,
+    ExtensionInstallPrompt::Delegate* delegate,
+    const ExtensionInstallPrompt::Prompt& prompt,
+    const ExtensionInstallPrompt::ShowDialogCallback& show_dialog_callback) {
   AutoConfirmForTest auto_confirm = CheckAutoConfirmCommandLineSwitch();
   if (auto_confirm != DO_NOT_SKIP) {
     DoAutoConfirm(auto_confirm, delegate);
     return;
   }
-  ShowExtensionInstallDialogImpl(parent, navigator, delegate, prompt);
+  DCHECK(!show_dialog_callback.is_null());
+  show_dialog_callback.Run(parent, navigator, delegate, prompt);

[Aaron Boodman, 2012/10/04 06:33:35]

Too much inversion-of-control. I can't remember which way is up anymore.

We have ExtensionInstallDialog which is badly named and not actually a dialog,
but is the business logic of getting ready to display the dialog.

Then we have ExtensionInstallPrompt which is the actual impl of showing the
prompt. Currently EIP varies at compile-time by platform. This change
essentially wants to make it vary at runtime too, but instead of changing it to
be a real runtime interface, you are passing an interface through it, so we have
this weird nested ioc thing going on.

One really simple thing that Mihai suggested in person today would be to pass a
NativeWidget or NativeWindow or something as a param to
ShowExtensionInstallDialog. If non-NULL, this is the native view that the UI
should be appended to. Otherwise, we create a dialog and put it in there.

However, if it turns out you need to do more custom stuff, then you might
actually need to vary the impl of the prompt at runtime. In that case, I think
the right answer is to change extension_install_dialog.* into an actual
interface, and then have implementations for the current different behaviors,
then pass instances of this class through the various layers of the cake.

Sorry this isn't easier, but this code definitely needs to be cleaned up, and
you're lucky modifier #436.

 }