Add list of Shared Modules to App Info Dialog

chrome/browser/ui/views/apps/app_info_dialog/app_info_summary_tab.cc

Index: chrome/browser/ui/views/apps/app_info_dialog/app_info_summary_tab.cc
diff --git a/chrome/browser/ui/views/apps/app_info_dialog/app_info_summary_tab.cc b/chrome/browser/ui/views/apps/app_info_dialog/app_info_summary_tab.cc
index 6c5cce2a6f39b61fe5aca2a7fbc1569ace23b285..5d41a80b1bc29ae0816a6c54f5d1af75a251617c 100644
--- a/chrome/browser/ui/views/apps/app_info_dialog/app_info_summary_tab.cc
+++ b/chrome/browser/ui/views/apps/app_info_dialog/app_info_summary_tab.cc
@@ -25,6 +25,7 @@
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_icon_set.h"
 #include "extensions/common/manifest_handlers/icons_handler.h"
+#include "extensions/common/manifest_handlers/shared_module_info.h"
 #include "grit/generated_resources.h"
 #include "net/base/url_util.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -260,6 +261,121 @@ bool AppInfoSummaryPanel::CanShowAppInWebStore() const {
   return app_->from_webstore();
 }
 
+// A small summary panel with a list of the app's imported modules, and a link
+// to each of their options pages.
+class AppInfoImportedModulesPanel : public views::View,

[sashab, 2014/06/03 07:08:01]

I chose to make this a View because it has a variable number of child views
created as a part of it's construction, so they can't be stored as member
variables (i.e. you can't separate the construction from the layout). I could
make this class smaller, moving stuff like the title out into the main layout,
but I'm happy either way.

[benwells, 2014/06/04 00:04:17]

As it is a recognizable chunk of the dialog, which is either all there or all
not, I think it makes sense as a separate view. So, sgtm.

+                                    public views::LinkListener {
+ public:
+  AppInfoImportedModulesPanel(Profile* profile,
+                              const extensions::Extension* app);
+  virtual ~AppInfoImportedModulesPanel();
+
+ private:
+  // Overridden from views::LinkListener:
+  virtual void LinkClicked(views::Link* source, int event_flags) OVERRIDE;
+
+  views::Label* title_;

[benwells, 2014/06/04 00:04:17]

I don't think this needs to be a field.

[sashab, 2014/06/05 00:27:35]

Done.

+
+  Profile* profile_;
+  const extensions::Extension* app_;
+  std::map<views::Link*, GURL> about_links_;
+
+  DISALLOW_COPY_AND_ASSIGN(AppInfoImportedModulesPanel);
+};
+
+AppInfoImportedModulesPanel::AppInfoImportedModulesPanel(
+    Profile* profile,
+    const extensions::Extension* app)
+    : title_(NULL), profile_(profile), app_(app) {
+  // Create controls.
+  title_ =
+      new views::Label(l10n_util::GetStringUTF16(
+                           IDS_APPLICATION_INFO_IMPORTED_MODULES_TITLE_TEXT),
+                       ui::ResourceBundle::GetSharedInstance().GetFontList(
+                           ui::ResourceBundle::BoldFont));
+  title_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+
+  // Layout elements.
+  views::GridLayout* layout = new views::GridLayout(this);
+  SetLayoutManager(layout);
+
+  static const int kTitleColumnId = 1;

[benwells, 2014/06/04 00:04:17]

Nit: these are column sets, not columns, so a name like kTitleColumnSetId would
be more appopriate.

[sashab, 2014/06/05 00:27:35]

Done.

+  views::ColumnSet* title_column_set = layout->AddColumnSet(kTitleColumnId);
+  title_column_set->AddColumn(views::GridLayout::LEADING,
+                              views::GridLayout::LEADING,
+                              1,  // Stretch the title to as wide as needed
+                              views::GridLayout::USE_PREF,
+                              0,
+                              0);
+
+  static const int kModulesListColumnId = 2;

[benwells, 2014/06/04 00:04:17]

Same comment here, kModulesColumnSetId would be more appropriate.

[sashab, 2014/06/05 00:27:35]

Done.

+  views::ColumnSet* column_set = layout->AddColumnSet(kModulesListColumnId);
+  column_set->AddColumn(views::GridLayout::LEADING,
+                        views::GridLayout::LEADING,
+                        1,  // Stretch the title to as wide as needed
+                        views::GridLayout::USE_PREF,
+                        0,
+                        0);
+  column_set->AddPaddingColumn(0, views::kRelatedControlSmallHorizontalSpacing);
+  column_set->AddColumn(views::GridLayout::LEADING,
+                        views::GridLayout::LEADING,
+                        0,  // Do not stretch the 'about' link
+                        views::GridLayout::USE_PREF,
+                        0,
+                        0);
+
+  layout->StartRow(0, kTitleColumnId);
+  layout->AddView(title_);
+
+  // Find all the shared modules for this app, and display them in a list.
+  // TODO(sashab): Since we only have 1 shared module at the moment, the

[benwells, 2014/06/04 00:04:17]

Nit: its a good idea not to say things like 'at the moment' in comments, as it
will be checked in and time goes on :)

I'd rephrase this to be something like: TODO(sashab): Revisit UI layout once
shared module usage becomes more common.

[sashab, 2014/06/05 00:27:35]

Done.

+  // maximum number of shared modules this app can have is 1. So this is a
+  // temporary solution but does not scale for multiple shared modules - a
+  // different UI element (or a scrollable dialog) needs to be used if multiple
+  // modules are needed.
+  ExtensionService* service =
+      extensions::ExtensionSystem::Get(profile_)->extension_service();
+  DCHECK(service);
+  const std::vector<extensions::SharedModuleInfo::ImportInfo>& imports =
+      extensions::SharedModuleInfo::GetImports(app_);
+  for (size_t i = 0; i < imports.size(); ++i) {
+    const extensions::Extension* imported_module =
+        service->GetExtensionById(imports[i].extension_id, true);
+    if (imported_module &&
+        extensions::SharedModuleInfo::IsSharedModule(imported_module) &&

[benwells, 2014/06/04 00:04:17]

I didn't think IsSharedModule or IsExportAllowedByWhitelist is needed here? This
is checked during extension install already, which is the right place to be
checking for it.

[sashab, 2014/06/05 00:27:35]

Ahh yes, I remember this discussion. *Technically* an shared module could still
change state (i.e. become disabled) while this dialog is open, in which case
GetExtensionById() would return NULL.

Update: Shared modules cannot be disabled. Changed to a DCHECK.

+        extensions::SharedModuleInfo::IsExportAllowedByWhitelist(
+            imported_module, app_->id())) {
+      views::Label* name_label =
+          new views::Label(base::UTF8ToUTF16(imported_module->name()));
+      name_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+
+      layout->StartRow(0, kModulesListColumnId);
+      layout->AddView(name_label);
+
+      // If this app has an options page, display it as an 'about' link.
+      GURL options_page =
+          extensions::ManifestURL::GetOptionsPage(imported_module);
+      if (options_page != GURL::EmptyGURL()) {
+        views::Link* about_link = new views::Link(l10n_util::GetStringUTF16(
+            IDS_APPLICATION_INFO_IMPORTED_MODULES_ABOUT_LINK_TEXT));
+        about_link->set_listener(this);
+        about_links_[about_link] = options_page;
+        layout->AddView(about_link);
+      }
+    }
+  }
+}
+
+AppInfoImportedModulesPanel::~AppInfoImportedModulesPanel() {
+}
+
+void AppInfoImportedModulesPanel::LinkClicked(views::Link* source,
+                                              int event_flags) {
+  chrome::NavigateParams params(
+      profile_, about_links_[source], content::PAGE_TRANSITION_LINK);
+  chrome::Navigate(&params);
+}
+
 }  // namespace
 
 // A model for a combobox selecting the launch options for a hosted app.
@@ -386,6 +502,9 @@ AppInfoSummaryTab::AppInfoSummaryTab(gfx::NativeWindow parent_window,
   if (launch_options_combobox_)
     AddChildView(launch_options_combobox_);
 
+  if (HasImportedModules())
+    AddChildView(new AppInfoImportedModulesPanel(profile_, app_));
+
   LayoutButtons();
 }
 
@@ -534,3 +653,7 @@ bool AppInfoSummaryTab::CanCreateShortcuts() const {
   return true;
 #endif
 }
+
+bool AppInfoSummaryTab::HasImportedModules() {
+  return extensions::SharedModuleInfo::ImportsModules(app_);
+}