Restore the permission selection listing in the Origin Info Bubble.

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/website_settings/website_settings_popup_view.h"
 
 #include "base/macros.h"
 #include "chrome/browser/ui/views/website_settings/chosen_object_view.h"
 #include "chrome/browser/ui/views/website_settings/permission_selector_view.h"
 #include "chrome/browser/usb/usb_chooser_context.h"
@@ skipping 146 matching lines @@
 }  // namespace
 
 // Test UI construction and reconstruction via
 // WebsiteSettingsPopupView::SetPermissionInfo().
 TEST_F(WebsiteSettingsPopupViewTest, SetPermissionInfo) {
   PermissionInfoList list(1);
   list.back().type = CONTENT_SETTINGS_TYPE_GEOLOCATION;
   list.back().source = content_settings::SETTING_SOURCE_USER;
   list.back().is_incognito = false;
 
-  EXPECT_EQ(0, api_->permissions_content()->child_count());
+#ifdef OS_MACOSX
+  // Mac OS X still has Fullscreen and Mouse Lock permissions.

[tapted, 2016/03/06 23:02:08]

This is actually in a fieldtrial on Mac, to catch up to the behaviour on other
platforms, and https://codereview.chromium.org/1709803004/ means that the bots
run with the fieldtrial enabled by default.

Maybe instead of the #ifdef, it should be a runtime check for
ExclusiveAccessManager::IsSimplifiedFullscreenUIEnabled()

or base::FeatureList::IsEnabled(features::kSimplifiedFullscreenUI); (if it's
needed outside of c/b/ui but that also needs mgiuca's
https://codereview.chromium.org/1763653002/ to land which is Android's edition
of kSimplifiedFullscreenUI). This would make it consistent with the
`ShouldShowPermission()` function in website_settings.cc's anonymous namepace.

Anyway, I think IsSimplifiedFullscreenUIEnabled() is the right fix, but it needs
to be applied in WebsiteSettingsPopupViewTest.SetPermissionInfoWithUsbDevice as
well as WebsiteSettingsPopupViewTest.SetPermissionInfo

However, looking at the failures, there does seem to be something else at play.
E.g. I think just using kExpectedChildren in SetPermissionInfoWithUsbDevice
would also "work". I think the problem is that this is a unittest, but
base::FeatureList::IsEnabled() only populates the contents of the fieldtrial
testing config in a browsertest. So
ExclusiveAccessManager::IsSimplifiedFullscreenUIEnabled() will still be
consistent, but it will be false on Mac where really it should be true (as it
would be in a browsertest).

Really there's some code in ChromeBrowserMainParts::SetupMetricsAndFieldTrials()
that should be exposed to unit tests, so that they can trigger a call
chrome_variations::AssociateDefaultFieldTrialConfig to get the same parameters
as a browsertest. But that's probably a yak that doesn't need shaving before
this.

[palmer, 2016/03/09 20:13:07]

OK, I've changed the tests to use
ExclusiveAccessManager::IsSimplifiedFullscreenUIEnabled(). Seems to work.
Thanks!

+  const int kExpectedChildren = 12;
+#else
+  const int kExpectedChildren = 10;
+#endif
+
+  EXPECT_EQ(kExpectedChildren, api_->permissions_content()->child_count());
 
   list.back().setting = CONTENT_SETTING_ALLOW;
   api_->SetPermissionInfo(list);
-  EXPECT_EQ(1, api_->permissions_content()->child_count());
+  EXPECT_EQ(kExpectedChildren, api_->permissions_content()->child_count());
 
   PermissionSelectorView* selector = api_->GetPermissionSelectorAt(0);
   EXPECT_EQ(3, selector->child_count());
 
   // 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("Allowed by you"),
             api_->GetPermissionButtonAt(0)->GetText());
 
   // Verify calling SetPermisisonInfo() directly updates the UI.
   list.back().setting = CONTENT_SETTING_BLOCK;
   api_->SetPermissionInfo(list);
   EXPECT_EQ(base::ASCIIToUTF16("Blocked by you"),
             api_->GetPermissionButtonAt(0)->GetText());
 
   // Simulate a user selection via the UI. Note this will also cover logic in
   // WebsiteSettings to update the pref.
   list.back().setting = CONTENT_SETTING_ALLOW;
   api_->GetPermissionSelectorAt(0)->PermissionChanged(list.back());
-  EXPECT_EQ(1, api_->permissions_content()->child_count());
+  EXPECT_EQ(kExpectedChildren, api_->permissions_content()->child_count());
   EXPECT_EQ(base::ASCIIToUTF16("Allowed by you"),
             api_->GetPermissionButtonAt(0)->GetText());
 
   // Setting to the default via the UI should keep the button around.
   list.back().setting = CONTENT_SETTING_ASK;
   api_->GetPermissionSelectorAt(0)->PermissionChanged(list.back());
-  EXPECT_EQ(1, api_->permissions_content()->child_count());
+  EXPECT_EQ(kExpectedChildren, api_->permissions_content()->child_count());
   EXPECT_EQ(base::ASCIIToUTF16("Ask by you"),
             api_->GetPermissionButtonAt(0)->GetText());
 
   // However, since the setting is now default, recreating the dialog with those
   // settings should omit the permission from the UI.
   api_->SetPermissionInfo(list);
-  EXPECT_EQ(0, api_->permissions_content()->child_count());
+  EXPECT_EQ(kExpectedChildren, api_->permissions_content()->child_count());
 }
 
 // Test UI construction and reconstruction with USB devices.
 TEST_F(WebsiteSettingsPopupViewTest, SetPermissionInfoWithUsbDevice) {
-  EXPECT_EQ(0, api_->permissions_content()->child_count());
+  EXPECT_EQ(10, api_->permissions_content()->child_count());
 
   const GURL origin = GURL(kUrl).GetOrigin();
   scoped_refptr<device::UsbDevice> device =
       new device::MockUsbDevice(0, 0, "Google", "Gizmo", "1234567890");
   device_client_.usb_service().AddDevice(device);
   UsbChooserContext* store =
       UsbChooserContextFactory::GetForProfile(web_contents_helper_.profile());
   store->GrantDevicePermission(origin, origin, device->guid());
 
   PermissionInfoList list;
   api_->SetPermissionInfo(list);
-  EXPECT_EQ(1, api_->permissions_content()->child_count());
+  EXPECT_EQ(11, api_->permissions_content()->child_count());
 
   ChosenObjectView* object_view =
-      static_cast<ChosenObjectView*>(api_->permissions_content()->child_at(0));
+      static_cast<ChosenObjectView*>(api_->permissions_content()->child_at(10));
   EXPECT_EQ(3, object_view->child_count());
 
   const int kLabelIndex = 1;
   views::Label* label =
       static_cast<views::Label*>(object_view->child_at(kLabelIndex));
   EXPECT_EQ(base::ASCIIToUTF16("Gizmo"), label->text());
 
   const int kButtonIndex = 2;
   views::Button* button =
       static_cast<views::Button*>(object_view->child_at(kButtonIndex));
 
   const ui::MouseEvent event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
                              ui::EventTimeForNow(), 0, 0);
   views::ButtonListener* button_listener =
       static_cast<views::ButtonListener*>(object_view);
   button_listener->ButtonPressed(button, event);
   api_->SetPermissionInfo(list);
-  EXPECT_EQ(0, api_->permissions_content()->child_count());
+  EXPECT_EQ(10, api_->permissions_content()->child_count());
   EXPECT_FALSE(store->HasDevicePermission(
       origin, origin, *device::usb::DeviceInfo::From(*device)));
 }