Fix tab sharing infobar layout when the shared tab is untitled.

Previously an empty shared tab name would indicate that the infobar
being constructed is for the shared tab.
- Layout for the shared tab:
Sharing this tab to |app_name|  [Stop]
- Layout for all other tabs:
Sharing |shared_tab_name| to |app_name| [Stop] [Share this tab instead]

However, sometimes the shared tab name can be empty (see bug), leading
to incorrect infobars layout. Don't rely on the shared tab name for
infobar construction, use a separate parameter. When the shared tab is
untitled, create infobars for non-shared tabs using this layout:
Sharing a tab to |app_name| [Stop] [Share this tab instead]

Bug: 1011379
Change-Id: Iedf288060d20b8a97a4c769d83a83e74ac057bd4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2023828
Commit-Queue: Marina Ciocea <marinaciocea@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736290}

chrome/browser/infobars/infobars_browsertest.cc

@@ -377,7 +377,7 @@ void InfoBarUiTest::ShowUi(const std::string& name) {
     case IBD::TAB_SHARING_INFOBAR_DELEGATE:
       TabSharingInfoBarDelegate::Create(
           GetInfoBarService(), base::ASCIIToUTF16("example.com"),
-          base::ASCIIToUTF16("application.com"), true, nullptr);
+          base::ASCIIToUTF16("application.com"), false, true, nullptr);
       break;
 
     default:

chrome/browser/ui/tab_sharing/tab_sharing_infobar_delegate.cc

@@ -18,22 +18,25 @@ infobars::InfoBar* TabSharingInfoBarDelegate::Create(
     InfoBarService* infobar_service,
     const base::string16& shared_tab_name,
     const base::string16& app_name,
-    bool is_sharing_allowed,
+    bool shared_tab,
+    bool can_share,
     TabSharingUI* ui) {
   DCHECK(infobar_service);
   return infobar_service->AddInfoBar(infobar_service->CreateConfirmInfoBar(
-      base::WrapUnique(new TabSharingInfoBarDelegate(shared_tab_name, app_name,
-                                                     is_sharing_allowed, ui))));
+      base::WrapUnique(new TabSharingInfoBarDelegate(
+          shared_tab_name, app_name, shared_tab, can_share, ui))));
 }
 
 TabSharingInfoBarDelegate::TabSharingInfoBarDelegate(
     base::string16 shared_tab_name,
     base::string16 app_name,
-    bool is_sharing_allowed,
+    bool shared_tab,
+    bool can_share,
     TabSharingUI* ui)
     : shared_tab_name_(std::move(shared_tab_name)),
       app_name_(std::move(app_name)),
-      is_sharing_allowed_(is_sharing_allowed),
+      shared_tab_(shared_tab),
+      can_share_(can_share),
       ui_(ui) {}
 
 bool TabSharingInfoBarDelegate::EqualsDelegate(
@@ -52,13 +55,17 @@ TabSharingInfoBarDelegate::GetIdentifier() const {
 }
 
 base::string16 TabSharingInfoBarDelegate::GetMessageText() const {
-  if (shared_tab_name_.empty()) {
+  if (shared_tab_) {
     return l10n_util::GetStringFUTF16(
         IDS_TAB_SHARING_INFOBAR_SHARING_CURRENT_TAB_LABEL, app_name_);
   }
-  return l10n_util::GetStringFUTF16(
-      IDS_TAB_SHARING_INFOBAR_SHARING_ANOTHER_TAB_LABEL, shared_tab_name_,
-      app_name_);
+  return !shared_tab_name_.empty()
+             ? l10n_util::GetStringFUTF16(
+                   IDS_TAB_SHARING_INFOBAR_SHARING_ANOTHER_TAB_LABEL,
+                   shared_tab_name_, app_name_)
+             : l10n_util::GetStringFUTF16(
+                   IDS_TAB_SHARING_INFOBAR_SHARING_ANOTHER_UNTITLED_TAB_LABEL,
+                   app_name_);
 }
 
 base::string16 TabSharingInfoBarDelegate::GetButtonLabel(
@@ -69,9 +76,7 @@ base::string16 TabSharingInfoBarDelegate::GetButtonLabel(
 }
 
 int TabSharingInfoBarDelegate::GetButtons() const {
-  return shared_tab_name_.empty() || !is_sharing_allowed_
-             ? BUTTON_OK
-             : BUTTON_OK | BUTTON_CANCEL;
+  return shared_tab_ || !can_share_ ? BUTTON_OK : BUTTON_OK | BUTTON_CANCEL;
 }
 
 bool TabSharingInfoBarDelegate::Accept() {

chrome/browser/ui/tab_sharing/tab_sharing_infobar_delegate.h

@@ -20,22 +20,26 @@ class TabSharingUI;
 // "Sharing this tab to |app_name_|  [Stop]"
 // Layout for all other tabs:
 // "Sharing |shared_tab_name_| to |app_name_| [Stop] [Share this tab instead]"
+// or if |shared_tab_name_| is empty:
+// "Sharing a tab to |app_name_| [Stop] [Share this tab instead]"
 class TabSharingInfoBarDelegate : public ConfirmInfoBarDelegate {
  public:
-  // Creates a tab sharing infobar. If |shared_tab_name| is empty, it creates an
+  // Creates a tab sharing infobar. If |shared_tab| is true, it creates an
   // infobar with "currently shared tab" layout (see class comment). If
-  // |is_sharing_allowed| is false, [Share this tab] button is not displayed.
+  // |can_share| is false, [Share this tab] button is not displayed.
   static infobars::InfoBar* Create(InfoBarService* infobar_service,
                                    const base::string16& shared_tab_name,
                                    const base::string16& app_name,
-                                   bool is_sharing_allowed,
+                                   bool shared_tab,
+                                   bool can_share,
                                    TabSharingUI* ui);
   ~TabSharingInfoBarDelegate() override = default;
 
  private:
   TabSharingInfoBarDelegate(base::string16 shared_tab_name,
                             base::string16 app_name,
-                            bool is_sharing_allowed,
+                            bool shared_tab,
+                            bool can_share,
                             TabSharingUI* ui);
 
   // ConfirmInfoBarDelegate:
@@ -52,7 +56,8 @@ class TabSharingInfoBarDelegate : public ConfirmInfoBarDelegate {
 
   const base::string16 shared_tab_name_;
   const base::string16 app_name_;
-  bool is_sharing_allowed_;
+  bool shared_tab_;
+  bool can_share_;
 
   // Creates and removes delegate's infobar; outlives delegate.
   TabSharingUI* ui_;

chrome/browser/ui/tab_sharing/tab_sharing_infobar_delegate_unittest.cc

@@ -42,20 +42,23 @@ class TabSharingInfoBarDelegateTest : public BrowserWithTestWindowTest {
 
   infobars::InfoBar* CreateInfobar(base::string16 shared_tab_name,
                                    base::string16 app_name,
-                                   bool is_sharing_allowed,
+                                   bool shared_tab,
+                                   bool can_share,
                                    int tab_index = 0) {
     return TabSharingInfoBarDelegate::Create(
         InfoBarService::FromWebContents(
             browser()->tab_strip_model()->GetWebContentsAt(tab_index)),
-        shared_tab_name, app_name, is_sharing_allowed, tab_sharing_mock_ui());
+        shared_tab_name, app_name, shared_tab, can_share,
+        tab_sharing_mock_ui());
   }
 
   ConfirmInfoBarDelegate* CreateDelegate(base::string16 shared_tab_name,
                                          base::string16 app_name,
-                                         bool is_sharing_allowed,
+                                         bool shared_tab,
+                                         bool can_share,
                                          int tab_index = 0) {
-    infobars::InfoBar* infobar =
-        CreateInfobar(shared_tab_name, app_name, is_sharing_allowed, tab_index);
+    infobars::InfoBar* infobar = CreateInfobar(
+        shared_tab_name, app_name, shared_tab, can_share, tab_index);
     return static_cast<ConfirmInfoBarDelegate*>(infobar->delegate());
   }
 
@@ -67,7 +70,8 @@ class TabSharingInfoBarDelegateTest : public BrowserWithTestWindowTest {
 
 TEST_F(TabSharingInfoBarDelegateTest, StartSharingOnCancel) {
   AddTab(browser(), GURL("about:blank"));
-  infobars::InfoBar* infobar = CreateInfobar(kSharedTabName, kAppName, true);
+  infobars::InfoBar* infobar =
+      CreateInfobar(kSharedTabName, kAppName, false, true);
   ConfirmInfoBarDelegate* delegate =
       static_cast<ConfirmInfoBarDelegate*>(infobar->delegate());
   EXPECT_CALL(*tab_sharing_mock_ui(), StartSharing(infobar)).Times(1);
@@ -77,7 +81,7 @@ TEST_F(TabSharingInfoBarDelegateTest, StartSharingOnCancel) {
 TEST_F(TabSharingInfoBarDelegateTest, StopSharingOnAccept) {
   AddTab(browser(), GURL("about:blank"));
   ConfirmInfoBarDelegate* delegate =
-      CreateDelegate(kSharedTabName, kAppName, true);
+      CreateDelegate(kSharedTabName, kAppName, false, true);
   EXPECT_CALL(*tab_sharing_mock_ui(), StopSharing).Times(1);
   EXPECT_FALSE(delegate->Accept());
 }
@@ -87,7 +91,7 @@ TEST_F(TabSharingInfoBarDelegateTest, StopSharingOnAccept) {
 TEST_F(TabSharingInfoBarDelegateTest, InfobarOnSharedTab) {
   AddTab(browser(), GURL("about:blank"));
   ConfirmInfoBarDelegate* delegate =
-      CreateDelegate(base::string16(), kAppName, true);
+      CreateDelegate(base::string16(), kAppName, true, true);
   EXPECT_STREQ(delegate->GetVectorIcon().name,
                vector_icons::kScreenShareIcon.name);
   EXPECT_EQ(delegate->GetMessageText(),
@@ -104,7 +108,7 @@ TEST_F(TabSharingInfoBarDelegateTest, InfobarOnSharedTab) {
 TEST_F(TabSharingInfoBarDelegateTest, InfobarOnNotSharedTab) {
   AddTab(browser(), GURL("about:blank"));
   ConfirmInfoBarDelegate* delegate =
-      CreateDelegate(kSharedTabName, kAppName, true);
+      CreateDelegate(kSharedTabName, kAppName, false, true);
   EXPECT_STREQ(delegate->GetVectorIcon().name,
                vector_icons::kScreenShareIcon.name);
   EXPECT_EQ(delegate->GetMessageText(),
@@ -126,14 +130,14 @@ TEST_F(TabSharingInfoBarDelegateTest, InfobarWhenSharingNotAllowed) {
   // Create infobar for shared tab.
   AddTab(browser(), GURL("about:blank"));
   ConfirmInfoBarDelegate* delegate_shared_tab = CreateDelegate(
-      base::string16(), kAppName, false /* is_sharing_allowed */, 0);
+      base::string16(), kAppName, true, false /* can_share */, 0);
   EXPECT_EQ(delegate_shared_tab->GetButtons(),
             ConfirmInfoBarDelegate::BUTTON_OK);
 
   // Create infobar for another not shared tab.
   AddTab(browser(), GURL("about:blank"));
-  ConfirmInfoBarDelegate* delegate = CreateDelegate(
-      kSharedTabName, kAppName, false /* is_sharing_allowed */, 1);
+  ConfirmInfoBarDelegate* delegate =
+      CreateDelegate(kSharedTabName, kAppName, false, false /* can_share */, 1);
   EXPECT_EQ(delegate->GetButtons(), ConfirmInfoBarDelegate::BUTTON_OK);
 }
 
@@ -143,9 +147,9 @@ TEST_F(TabSharingInfoBarDelegateTest, MultipleInfobarsOnSameTab) {
   InfoBarService* infobar_service = InfoBarService::FromWebContents(
       browser()->tab_strip_model()->GetWebContentsAt(0));
   EXPECT_EQ(infobar_service->infobar_count(), 0u);
-  CreateInfobar(kSharedTabName, kAppName, true);
+  CreateInfobar(kSharedTabName, kAppName, false, true);
   EXPECT_EQ(infobar_service->infobar_count(), 1u);
-  CreateInfobar(kSharedTabName, kAppName, true);
+  CreateInfobar(kSharedTabName, kAppName, false, true);
   EXPECT_EQ(infobar_service->infobar_count(), 2u);
 }
 
@@ -155,7 +159,7 @@ TEST_F(TabSharingInfoBarDelegateTest, InfobarNotDismissedOnNavigation) {
       browser()->tab_strip_model()->GetWebContentsAt(0);
   InfoBarService* infobar_service =
       InfoBarService::FromWebContents(web_contents);
-  CreateInfobar(kSharedTabName, kAppName, true);
+  CreateInfobar(kSharedTabName, kAppName, false, true);
   EXPECT_EQ(infobar_service->infobar_count(), 1u);
   content::NavigationController* controller = &web_contents->GetController();
   NavigateAndCommit(controller, GURL("http://bar"));

chrome/browser/ui/views/tab_sharing/tab_sharing_ui_views.cc

@@ -276,9 +276,9 @@ void TabSharingUIViews::CreateInfobarForWebContents(
   auto* infobar_service = InfoBarService::FromWebContents(contents);
   infobar_service->AddObserver(this);
   infobars_[contents] = TabSharingInfoBarDelegate::Create(
-      infobar_service,
-      shared_tab_ == contents ? base::string16() : shared_tab_name_, app_name_,
-      !source_callback_.is_null() /*is_sharing_allowed*/, this);
+      infobar_service, shared_tab_name_, app_name_,
+      shared_tab_ == contents /*shared_tab*/,
+      !source_callback_.is_null() /*can_share*/, this);
 }
 
 void TabSharingUIViews::RemoveInfobarsForAllTabs() {