Add new DialogDelegate for dialogs in the App List

chrome/browser/ui/app_list/app_list_controller_delegate.cc

Index: chrome/browser/ui/app_list/app_list_controller_delegate.cc
diff --git a/chrome/browser/ui/app_list/app_list_controller_delegate.cc b/chrome/browser/ui/app_list/app_list_controller_delegate.cc
index c174a07b22fed69a06a292b48076d184beff0ac0..754ccb1a8cdfec43219d60e86fd034bf56239626 100644
--- a/chrome/browser/ui/app_list/app_list_controller_delegate.cc
+++ b/chrome/browser/ui/app_list/app_list_controller_delegate.cc
@@ -105,19 +105,8 @@ void AppListControllerDelegate::DoShowAppInfoFlow(
   if (!parent_window)
     return;
 
-  // For the centered app list, inset the dialog further so it appears as a
-  // vertical column in the center of the app list.
-  const int kAppListDialogHorizontalBorderInsets =
-      app_list::switches::IsCenteredAppListEnabled() ? 110 : 10;
-  const int kAppListDialogVerticalBorderInsets = 10;
-
-  gfx::Rect dialog_bounds = GetAppListBounds();
-  dialog_bounds.Inset(kAppListDialogHorizontalBorderInsets,
-                      kAppListDialogVerticalBorderInsets);
-
   OnShowChildDialog();
   ShowAppInfoDialog(parent_window,
-                    dialog_bounds,
                     profile,
                     extension,
                     base::Bind(&AppListControllerDelegate::OnCloseChildDialog,

[tapted, 2014/06/16 01:05:43]

Since it's now fetching the controller delegate in
AppListDialogContentsView::CreateDialogWidget, this feels a little unbalanced.
And, for example, a pointer to AppListControllerDelegate* is currently being
indirectly passed in to the function via this Closure, so it seems unnecessary
for CreateDialogWidget to fetch this pointer again - it just makes the
dependency in this closure harder to find.

Instead of this closure, I would just pass in the AppListControllerDelegate* -
it's a leaky singleton, so holding on to a pointer is OK (and it's essentially
what base::Unretained is doing anyway, so this makes this ownership a lot
clearer).

AppListDialogContentsView can then take this pointer in its constructor, and
AppInfoDialog::WindowClosing() can move up into
AppListDialogContentsView::WindowClosing and just be 

void AppListDialogContentsView::WindowClosing() {
  app_list_controller_delegate_->OnCloseChildDialog();
}


and theeen.... AppInfoDialog is no longer doing very much :/. I actually missed
that it was inheriting from AppListDialogContentsView rather than just being a
subview. I think it's unusual for more than one views::View subclasses in the
same inheritance chain to both be adding subviews.

If you want to keep the AppInfoDialog and AppListDialogContentsView separate,
AppInfoDialog should be a subview of AppListDialogContentsView rather than a
child class (i.e. AppInfoDialog should inherit from views::View and just be a
simple container for the tabbed pane). You could try this out by passing
AppInfoDialog* into the AppListDialogContentsView() constructor (along with the
app_list_controller_delegate_);

ShowAppInfoDialog(..) would then look something like 

  views::View* app_info_view = new AppInfoDialog(parent_window, profile, app);
  views::Widget* dialog_widget = AppListDialogContentsView::CreateDialogWidget(
    new AppListDialogContentsView(app_list_controller_delegate_,
app_info_view));
  dialog_widget->Show();


AppInfoTab and its subclasses don't seem to use close_callback - it should be
removed from the constructors.


It might also be neater to merge the constructors of AppListDialogContentsView
and AppInfoDialog and just have one class.