media picker: don't use TabbedPane for one source type

This change causes the desktop media picker not to use TabbedPane
unless there are multiple source types to switch between - otherwise
it presents an odd single-tab TabbedPane. Specifically, this change:

1) Renames the pane_ member of DesktopMediaPickerDialogView and makes
   it optional;
2) Refactors the constructor of that class to add the created views
   children of the TabbedPane only when there are >1, and otherwise as
   direct children of the DialogView itself;
3) Overrides GetClassName on that class to aid debugging/inspection;
4) Adds new strings (from discussion with UX) for the dialog title when
   showing only a single source type;
5) Has DesktopMediaPickerDialogView::GetWindowTitle() use those new
   strings when appropriate;
6) Makes DesktopMediaPickerViewsBrowserTest's InvokeUi test fill
   itself with some test data, as an aid to screenshots;
7) Adds a new InvokeUi test to that suite, named "tabs", which shows
   the UI with only a single source;
8) Adds a new test to that suite, SingleSourceTypeChangesTitle, which
   validates the changes from (4) & (5);
9) Overrides DesktopMediaTabList::GetHeightForWidth, which is required
   for it to lay out properly when contained in a BoxLayout directly
   rather than a TabbedPane;

To test this change, use the same steps as CL 1626010, or use InvokeUi.

Bug: 964332
Change-Id: I0ba7f47613cfbd76152e7ecbdad34b91230b603c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1627919
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Auto-Submit: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663173}

chrome/app/chromium_strings.grd

@@ -1264,6 +1264,9 @@ Please check your email at <ph name="ACCOUNT_EMAIL">$2<ex>jane.doe@example.com</
           Close
         </message>
       </if>
+      <message name="IDS_DESKTOP_MEDIA_PICKER_TITLE_WEB_CONTENTS_ONLY" desc="Title for the window picker dialog shown when desktop capture of a tab is requested by an app.">
+        Share a Chromium tab
+      </message>
     </messages>
   </release>
 </grit>

chrome/app/generated_resources.grd

@@ -8741,6 +8741,12 @@ Please help our engineers fix this problem. Tell us what happened right before y
       <message name="IDS_DESKTOP_MEDIA_PICKER_TITLE" desc="Title for the window picker dialog shown when desktop capture is requested by an app.">
         Share your screen
       </message>
+      <message name="IDS_DESKTOP_MEDIA_PICKER_TITLE_SCREEN_ONLY" desc="Title for the window picker dialog shown when desktop capture of the entire screen is requested by an app.">
+        Share your entire screen
+      </message>
+      <message name="IDS_DESKTOP_MEDIA_PICKER_TITLE_WINDOW_ONLY" desc="Title for the window picker dialog shown when desktop capture of an application window is requested by an app.">
+        Share an application window
+      </message>
       <message name="IDS_DESKTOP_MEDIA_PICKER_TEXT" desc="Text for the window picker dialog shown when desktop capture is requested by an app to be used by the app itself.">
         <ph name="APP_NAME">$1<ex>Google Hangouts</ex></ph> wants to share the contents of your screen. Choose what you'd like to share.
       </message>

chrome/app/google_chrome_strings.grd

@@ -1284,6 +1284,9 @@ Please check your email at <ph name="ACCOUNT_EMAIL">$2<ex>jane.doe@example.com</
           Close
         </message>
       </if>
+      <message name="IDS_DESKTOP_MEDIA_PICKER_TITLE_WEB_CONTENTS_ONLY" desc="Title for the window picker dialog shown when desktop capture of a tab is requested by an app.">
+        Share a Chrome tab
+      </message>
     </messages>
   </release>
 </grit>

chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.cc

@@ -62,7 +62,7 @@ DesktopMediaPickerDialogView::DesktopMediaPickerDialogView(
       modality_(params.modality),
       description_label_(new views::Label()),
       audio_share_checkbox_(nullptr),
-      pane_(new views::TabbedPane()) {
+      tabbed_pane_(nullptr) {
   const ChromeLayoutProvider* provider = ChromeLayoutProvider::Get();
 
   SetLayoutManager(std::make_unique<views::BoxLayout>(
@@ -74,6 +74,8 @@ DesktopMediaPickerDialogView::DesktopMediaPickerDialogView(
   description_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   AddChildView(description_label_);
 
+  std::vector<std::pair<base::string16, std::unique_ptr<View>>> panes;
+
   for (auto& source_list : source_lists) {
     switch (source_list->GetMediaListType()) {
       case DesktopMediaID::TYPE_NONE: {
@@ -118,8 +120,8 @@ DesktopMediaPickerDialogView::DesktopMediaPickerDialogView(
             kGenericScreenStyle.item_size.height() * 2);
         screen_scroll_view->set_hide_horizontal_scrollbar(true);
 
-        pane_->AddTab(screen_title_text, screen_scroll_view.release());
-        pane_->set_listener(this);
+        panes.push_back(
+            std::make_pair(screen_title_text, std::move(screen_scroll_view)));
         break;
       }
       case DesktopMediaID::TYPE_WINDOW: {
@@ -149,8 +151,8 @@ DesktopMediaPickerDialogView::DesktopMediaPickerDialogView(
                                          kWindowStyle.item_size.height() * 2);
         window_scroll_view->set_hide_horizontal_scrollbar(true);
 
-        pane_->AddTab(window_title_text, window_scroll_view.release());
-        pane_->set_listener(this);
+        panes.push_back(
+            std::make_pair(window_title_text, std::move(window_scroll_view)));
         break;
       }
       case DesktopMediaID::TYPE_WEB_CONTENTS: {
@@ -160,16 +162,26 @@ DesktopMediaPickerDialogView::DesktopMediaPickerDialogView(
             l10n_util::GetStringUTF16(IDS_DESKTOP_MEDIA_PICKER_SOURCE_TYPE_TAB);
         auto list_controller = std::make_unique<DesktopMediaListController>(
             this, std::move(source_list));
-        pane_->AddTab(title,
-                      list_controller->CreateTabListView(title).release());
+        panes.push_back(
+            std::make_pair(title, list_controller->CreateTabListView(title)));
         list_controllers_.push_back(std::move(list_controller));
 
-        pane_->set_listener(this);
         break;
       }
     }
   }
 
+  if (panes.size() > 1) {
+    auto tabbed_pane = std::make_unique<views::TabbedPane>();
+    for (auto& pane : panes)
+      tabbed_pane->AddTab(pane.first, pane.second.release());
+    tabbed_pane->set_listener(this);
+    tabbed_pane->SetFocusBehavior(views::View::FocusBehavior::NEVER);
+    tabbed_pane_ = AddChildView(std::move(tabbed_pane));
+  } else {
+    AddChildView(std::move(panes.front().second));
+  }
+
   if (params.app_name == params.target_name) {
     description_label_->SetText(l10n_util::GetStringFUTF16(
         IDS_DESKTOP_MEDIA_PICKER_TEXT, params.app_name));
@@ -180,8 +192,6 @@ DesktopMediaPickerDialogView::DesktopMediaPickerDialogView(
   }
 
   DCHECK(!source_types_.empty());
-  pane_->SetFocusBehavior(views::View::FocusBehavior::NEVER);
-  AddChildView(pane_);
 
   if (params.request_audio) {
     audio_share_checkbox_ = new views::Checkbox(
@@ -278,12 +288,14 @@ void DesktopMediaPickerDialogView::OnSourceTypeSwitched(int index) {
 
 const DesktopMediaListController*
 DesktopMediaPickerDialogView::GetSelectedController() const {
-  return list_controllers_[pane_->GetSelectedTabIndex()].get();
+  int index = tabbed_pane_ ? tabbed_pane_->GetSelectedTabIndex() : 0;
+  return list_controllers_[index].get();
 }
 
 DesktopMediaListController*
 DesktopMediaPickerDialogView::GetSelectedController() {
-  return list_controllers_[pane_->GetSelectedTabIndex()].get();
+  int index = tabbed_pane_ ? tabbed_pane_->GetSelectedTabIndex() : 0;
+  return list_controllers_[index].get();
 }
 
 void DesktopMediaPickerDialogView::DetachParent() {
@@ -295,12 +307,34 @@ gfx::Size DesktopMediaPickerDialogView::CalculatePreferredSize() const {
   return gfx::Size(kDialogViewWidth, GetHeightForWidth(kDialogViewWidth));
 }
 
+const char* DesktopMediaPickerDialogView::GetClassName() const {
+  return "DesktopMediaPickerDialogView";
+}
+
 ui::ModalType DesktopMediaPickerDialogView::GetModalType() const {
   return modality_;
 }
 
 base::string16 DesktopMediaPickerDialogView::GetWindowTitle() const {
-  return l10n_util::GetStringUTF16(IDS_DESKTOP_MEDIA_PICKER_TITLE);
+  int title_id = IDS_DESKTOP_MEDIA_PICKER_TITLE;
+
+  if (!tabbed_pane_) {
+    switch (source_types_.front()) {
+      case DesktopMediaID::TYPE_SCREEN:
+        title_id = IDS_DESKTOP_MEDIA_PICKER_TITLE_SCREEN_ONLY;
+        break;
+      case DesktopMediaID::TYPE_WINDOW:
+        title_id = IDS_DESKTOP_MEDIA_PICKER_TITLE_WINDOW_ONLY;
+        break;
+      case DesktopMediaID::TYPE_WEB_CONTENTS:
+        title_id = IDS_DESKTOP_MEDIA_PICKER_TITLE_WEB_CONTENTS_ONLY;
+        break;
+      default:
+        break;
+    }
+  }
+
+  return l10n_util::GetStringUTF16(title_id);
 }
 
 bool DesktopMediaPickerDialogView::IsDialogButtonEnabled(
@@ -310,7 +344,7 @@ bool DesktopMediaPickerDialogView::IsDialogButtonEnabled(
 }
 
 views::View* DesktopMediaPickerDialogView::GetInitiallyFocusedView() {
-  return list_controllers_[0]->GetViewForInitialFocus();
+  return list_controllers_.front()->GetViewForInitialFocus();
 }
 
 int DesktopMediaPickerDialogView::GetDefaultDialogButton() const {
@@ -396,9 +430,12 @@ void DesktopMediaPickerDialogView::AcceptSpecificSource(DesktopMediaID source) {
 
 void DesktopMediaPickerDialogView::SelectTab(
     content::DesktopMediaID::Type source_type) {
+  if (!tabbed_pane_)
+    return;
+
   for (size_t i = 0; i < source_types_.size(); i++) {
     if (source_types_[i] == source_type) {
-      pane_->SelectTabAt(i);
+      tabbed_pane_->SelectTabAt(i);
       return;
     }
   }

chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.h

@@ -42,10 +42,9 @@ class DesktopMediaPickerDialogView : public views::DialogDelegateView,
   // views::TabbedPaneListener:
   void TabSelectedAt(int index) override;
 
-  // views::View:
-  gfx::Size CalculatePreferredSize() const override;
-
   // views::DialogDelegateView:
+  gfx::Size CalculatePreferredSize() const override;
+  const char* GetClassName() const override;
   ui::ModalType GetModalType() const override;
   base::string16 GetWindowTitle() const override;
   bool IsDialogButtonEnabled(ui::DialogButton button) const override;
@@ -72,7 +71,7 @@ class DesktopMediaPickerDialogView : public views::DialogDelegateView,
 
   views::Checkbox* audio_share_checkbox_;
 
-  views::TabbedPane* pane_;
+  views::TabbedPane* tabbed_pane_;
   std::vector<std::unique_ptr<DesktopMediaListController>> list_controllers_;
   std::vector<content::DesktopMediaID::Type> source_types_;
 

chrome/browser/ui/views/desktop_capture/desktop_media_picker_views_browsertest.cc

@@ -7,14 +7,19 @@
 #include <memory>
 #include <string>
 
+#include "base/callback.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/bind_test_util.h"
 #include "chrome/browser/media/webrtc/desktop_media_list.h"
 #include "chrome/browser/media/webrtc/fake_desktop_media_list.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/test/test_browser_dialog.h"
+#include "chrome/grit/chromium_strings.h"
+#include "chrome/grit/generated_resources.h"
 #include "content/public/browser/desktop_media_id.h"
+#include "ui/base/l10n/l10n_util.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/window/dialog_client_view.h"
 #include "ui/views/window/dialog_delegate.h"
@@ -29,32 +34,98 @@ class DesktopMediaPickerViewsBrowserTest : public DialogBrowserTest {
     auto* web_contents = browser()->tab_strip_model()->GetActiveWebContents();
     gfx::NativeWindow native_window = browser()->window()->GetNativeWindow();
 
-    std::vector<std::unique_ptr<DesktopMediaList>> source_lists;
-    for (auto type : {content::DesktopMediaID::TYPE_SCREEN,
-                      content::DesktopMediaID::TYPE_WINDOW,
-                      content::DesktopMediaID::TYPE_WEB_CONTENTS}) {
-      source_lists.push_back(std::make_unique<FakeDesktopMediaList>(type));
+    std::vector<std::unique_ptr<DesktopMediaList>> sources;
+    if (override_source_lists_.empty()) {
+      sources = CreateDefaultSourceLists();
+    } else {
+      for (auto& source : override_source_lists_)
+        sources.push_back(std::move(source));
     }
 
+    std::vector<FakeDesktopMediaList*> source_lists;
+    for (const auto& source : sources)
+      source_lists.push_back(static_cast<FakeDesktopMediaList*>(source.get()));
+
     DesktopMediaPicker::Params picker_params;
     picker_params.web_contents = web_contents;
     picker_params.context = native_window;
     picker_params.app_name = base::ASCIIToUTF16("app_name");
     picker_params.target_name = base::ASCIIToUTF16("target_name");
     picker_params.request_audio = true;
-    picker_->Show(picker_params, std::move(source_lists),
+    picker_->Show(picker_params, std::move(sources),
                   DesktopMediaPicker::DoneCallback());
+
+    if (after_show_callback_)
+      std::move(after_show_callback_).Run(source_lists);
   }
 
  protected:
+  std::vector<std::unique_ptr<DesktopMediaList>> CreateDefaultSourceLists() {
+    std::vector<std::unique_ptr<DesktopMediaList>> sources;
+    for (auto type : {content::DesktopMediaID::TYPE_SCREEN,
+                      content::DesktopMediaID::TYPE_WINDOW,
+                      content::DesktopMediaID::TYPE_WEB_CONTENTS}) {
+      sources.push_back(std::make_unique<FakeDesktopMediaList>(type));
+    }
+    return sources;
+  }
+
   std::unique_ptr<DesktopMediaPickerViews> picker_;
 
+  // If this list isn't filled in, a default list of source lists will be
+  // created.
+  std::vector<std::unique_ptr<FakeDesktopMediaList>> override_source_lists_;
+
+  // This callback is called in ShowUi after the picker dialog has been shown.
+  // This both more closely mirrors how this code behaves in production (where
+  // the DesktopMediaList is filled asynchronously, so it starts off empty and
+  // then becomes filled after the UI is showing) and allows InvokeUi-style
+  // tests to update the UI state after showing it.
+  base::OnceCallback<void(const std::vector<FakeDesktopMediaList*>&)>
+      after_show_callback_;
+
   DISALLOW_COPY_AND_ASSIGN(DesktopMediaPickerViewsBrowserTest);
 };
 
 // Invokes a dialog that allows the user to select what view of their desktop
 // they would like to share.
 IN_PROC_BROWSER_TEST_F(DesktopMediaPickerViewsBrowserTest, InvokeUi_default) {
+  after_show_callback_ =
+      base::BindOnce([](const std::vector<FakeDesktopMediaList*>& sources) {
+        sources[0]->AddSource(0);
+
+        // Fill in a bit of test data for nicer UI screenshots :)
+        sources[1]->AddSource(0);
+        sources[1]->SetSourceName(0, base::ASCIIToUTF16("Warty Warthog"));
+        sources[1]->AddSource(1);
+        sources[1]->SetSourceName(1, base::ASCIIToUTF16("Hoary Hedgehog"));
+        sources[1]->AddSource(2);
+        sources[1]->SetSourceName(2, base::ASCIIToUTF16("Breezy Badger"));
+
+        sources[2]->AddSource(0);
+        sources[2]->SetSourceName(0, base::ASCIIToUTF16("Dapper Drake"));
+        sources[2]->AddSource(1);
+        sources[2]->SetSourceName(1, base::ASCIIToUTF16("Edgy Eft"));
+        sources[2]->AddSource(2);
+        sources[2]->SetSourceName(2, base::ASCIIToUTF16("Feisty Fawn"));
+      });
+  ShowAndVerifyUi();
+}
+
+// Show the picker UI with only one source type: TYPE_WEB_CONTENTS, aka the
+// tab picker.
+IN_PROC_BROWSER_TEST_F(DesktopMediaPickerViewsBrowserTest, InvokeUi_tabs) {
+  after_show_callback_ =
+      base::BindOnce([](const std::vector<FakeDesktopMediaList*>& sources) {
+        sources[0]->AddSource(0);
+        sources[0]->SetSourceName(0, base::ASCIIToUTF16("Dapper Drake"));
+        sources[0]->AddSource(1);
+        sources[0]->SetSourceName(1, base::ASCIIToUTF16("Edgy Eft"));
+        sources[0]->AddSource(2);
+        sources[0]->SetSourceName(2, base::ASCIIToUTF16("Feisty Fawn"));
+      });
+  override_source_lists_.push_back(std::make_unique<FakeDesktopMediaList>(
+      content::DesktopMediaID::TYPE_WEB_CONTENTS));
   ShowAndVerifyUi();
 }
 
@@ -68,3 +139,16 @@ IN_PROC_BROWSER_TEST_F(DesktopMediaPickerViewsBrowserTest,
   EXPECT_FALSE(picker_->GetDialogViewForTesting()->IsDialogButtonEnabled(
       ui::DIALOG_BUTTON_OK));
 }
+
+// Validate that the dialog title changes to match the source type when there's
+// only one source type present.
+IN_PROC_BROWSER_TEST_F(DesktopMediaPickerViewsBrowserTest,
+                       SingleSourceTypeChangesTitle) {
+  override_source_lists_.push_back(std::make_unique<FakeDesktopMediaList>(
+      content::DesktopMediaID::TYPE_WEB_CONTENTS));
+  ShowUi(std::string());
+
+  EXPECT_EQ(picker_->GetDialogViewForTesting()->GetWindowTitle(),
+            l10n_util::GetStringUTF16(
+                IDS_DESKTOP_MEDIA_PICKER_TITLE_WEB_CONTENTS_ONLY));
+}

chrome/browser/ui/views/desktop_capture/desktop_media_picker_views_test_api.cc

@@ -73,7 +73,10 @@ void DesktopMediaPickerViewsTestApi::SelectTabForSourceType(
   const auto i =
       std::find(source_types.cbegin(), source_types.cend(), source_type);
   DCHECK(i != source_types.cend());
-  picker_->dialog_->pane_->SelectTabAt(std::distance(source_types.cbegin(), i));
+  if (picker_->dialog_->tabbed_pane_) {
+    picker_->dialog_->tabbed_pane_->SelectTabAt(
+        std::distance(source_types.cbegin(), i));
+  }
 }
 
 base::Optional<int> DesktopMediaPickerViewsTestApi::GetSelectedSourceId()

chrome/browser/ui/views/desktop_capture/desktop_media_tab_list.cc

@@ -165,6 +165,15 @@ gfx::Size DesktopMediaTabList::CalculatePreferredSize() const {
   return gfx::Size(0, rows * child_->row_height());
 }
 
+int DesktopMediaTabList::GetHeightForWidth(int width) const {
+  // If this method isn't overridden here, the default implementation would fall
+  // back to FillLayout's GetHeightForWidth, which would ask the TableView,
+  // which would return something based on the total number of rows, since
+  // TableView expects to always be sized by its container. Avoid even asking it
+  // by using the same height as CalculatePreferredSize().
+  return CalculatePreferredSize().height();
+}
+
 base::Optional<content::DesktopMediaID> DesktopMediaTabList::GetSelection() {
   int row = child_->FirstSelectedRow();
   if (row == -1)

chrome/browser/ui/views/desktop_capture/desktop_media_tab_list.h

@@ -44,6 +44,7 @@ class DesktopMediaTabList : public DesktopMediaListController::ListView {
   // views::View:
   const char* GetClassName() const override;
   gfx::Size CalculatePreferredSize() const override;
+  int GetHeightForWidth(int width) const override;
 
   // DesktopMediaListController::ListView:
   base::Optional<content::DesktopMediaID> GetSelection() override;