views: align columns in site settings dialog

chrome/browser/ui/views/website_settings/website_settings_popup_view_unittest.cc

Index: chrome/browser/ui/views/website_settings/website_settings_popup_view_unittest.cc
diff --git a/chrome/browser/ui/views/website_settings/website_settings_popup_view_unittest.cc b/chrome/browser/ui/views/website_settings/website_settings_popup_view_unittest.cc
index 6b6db780ed0ba1f45424f1244560baabdb29c057..b8ce7bd836f11393587eee0afb05460a9356a5dc 100644
--- a/chrome/browser/ui/views/website_settings/website_settings_popup_view_unittest.cc
+++ b/chrome/browser/ui/views/website_settings/website_settings_popup_view_unittest.cc
@@ -57,12 +57,18 @@ class WebsiteSettingsPopupViewTestApi {
 
   PermissionSelectorRow* GetPermissionSelectorAt(int index) {
     return static_cast<PermissionSelectorRow*>(

[sky, 2017/03/07 22:03:40]

Do you need this cast?

[Elly Fong-Jones, 2017/03/08 18:41:09]

Nope, deleted.

-        permissions_view()->child_at(index));
+        view_->selector_rows_.at(index).get());

[sky, 2017/03/07 22:03:40]

.at(index) -> [index] ?

[Elly Fong-Jones, 2017/03/08 18:41:09]

Done.

+  }
+
+  base::string16 GetPermissionLabelTextAt(int index) {
+    views::View* view = GetPermissionSelectorAt(index)->label_;
+    if (view->GetClassName() == views::Label::kViewClassName)

[sky, 2017/03/07 22:03:40]

This is subtle and worth a comment.

[Elly Fong-Jones, 2017/03/08 18:41:09]

Done.

+      return static_cast<views::Label*>(view)->text();
+    return base::string16();
   }
 
   base::string16 GetPermissionButtonTextAt(int index) {
-    const int kButtonIndex = 2;  // Button should be the third child.
-    views::View* view = GetPermissionSelectorAt(index)->child_at(kButtonIndex);
+    views::View* view = GetPermissionSelectorAt(index)->button();
     if (view->GetClassName() == views::MenuButton::kViewClassName) {
       return static_cast<views::MenuButton*>(view)->GetText();
     } else if (view->GetClassName() == views::Combobox::kViewClassName) {
@@ -175,7 +181,7 @@ TEST_F(WebsiteSettingsPopupViewTest, MAYBE_SetPermissionInfo) {
   list.back().setting = CONTENT_SETTING_DEFAULT;
 
   const int kExpectedChildren =
-      ExclusiveAccessManager::IsSimplifiedFullscreenUIEnabled() ? 11 : 13;
+      3 * (ExclusiveAccessManager::IsSimplifiedFullscreenUIEnabled() ? 11 : 13);

[sky, 2017/03/07 22:03:40]

The 3 is subtle and worth a comment. I suggest a constant that is shared with
the 3 below that has the comment.

[Elly Fong-Jones, 2017/03/08 18:41:09]

Done.

   EXPECT_EQ(kExpectedChildren, api_->permissions_view()->child_count());
 
   list.back().setting = CONTENT_SETTING_ALLOW;
@@ -183,15 +189,10 @@ TEST_F(WebsiteSettingsPopupViewTest, MAYBE_SetPermissionInfo) {
   EXPECT_EQ(kExpectedChildren, api_->permissions_view()->child_count());
 
   PermissionSelectorRow* selector = api_->GetPermissionSelectorAt(0);
-  EXPECT_EQ(3, selector->child_count());
+  EXPECT_NE(nullptr, selector);

[sky, 2017/03/07 22:03:40]

EXPECT(selector)?

[Elly Fong-Jones, 2017/03/08 18:41:09]

Done.

 
   // Verify labels match the settings on the PermissionInfoList.
-  const int kLabelIndex = 1;
-  EXPECT_EQ(views::Label::kViewClassName,
-            selector->child_at(kLabelIndex)->GetClassName());
-  views::Label* label =
-      static_cast<views::Label*>(selector->child_at(kLabelIndex));
-  EXPECT_EQ(base::ASCIIToUTF16("Location"), label->text());
+  EXPECT_EQ(base::ASCIIToUTF16("Location"), api_->GetPermissionLabelTextAt(0));
   EXPECT_EQ(base::ASCIIToUTF16("Allow"), api_->GetPermissionButtonTextAt(0));
 
   // Verify calling SetPermisisonInfo() directly updates the UI.
@@ -221,7 +222,7 @@ TEST_F(WebsiteSettingsPopupViewTest, MAYBE_SetPermissionInfo) {
 // Test UI construction and reconstruction with USB devices.
 TEST_F(WebsiteSettingsPopupViewTest, SetPermissionInfoWithUsbDevice) {
   const int kExpectedChildren =
-      ExclusiveAccessManager::IsSimplifiedFullscreenUIEnabled() ? 11 : 13;
+      3 * (ExclusiveAccessManager::IsSimplifiedFullscreenUIEnabled() ? 11 : 13);
   EXPECT_EQ(kExpectedChildren, api_->permissions_view()->child_count());
 
   const GURL origin = GURL(kUrl).GetOrigin();