PS - Filtering activeTab URL

chrome/browser/chromeos/extensions/extension_tab_util_delegate_chromeos_unittest.cc

Index: chrome/browser/chromeos/extensions/extension_tab_util_delegate_chromeos_unittest.cc
diff --git a/chrome/browser/chromeos/extensions/extension_tab_util_delegate_chromeos_unittest.cc b/chrome/browser/chromeos/extensions/extension_tab_util_delegate_chromeos_unittest.cc
index 9ded86efc10967c5cc86e16b1e69f743f309dfef..7b441e387d950ac2feaed31cfc82009e409be8aa 100644
--- a/chrome/browser/chromeos/extensions/extension_tab_util_delegate_chromeos_unittest.cc
+++ b/chrome/browser/chromeos/extensions/extension_tab_util_delegate_chromeos_unittest.cc
@@ -13,6 +13,10 @@ namespace extensions {
 
 namespace {
 
+const char kWhitelistedId[] = "cbkkbcmdlboombapidmoeolnmdacpkch";
+// Use an extension ID that will never be whitelisted.
+const char kNonWhitelistedId[] = "bogus";
+
 const char kTestUrl[] = "http://www.foo.bar/baz?key=val";
 const char kFilteredUrl[] = "http://www.foo.bar/";
 
@@ -30,21 +34,46 @@ void ExtensionTabUtilDelegateChromeOSTest::SetUp() {
   tab_.url.reset(new std::string(kTestUrl));
 }
 
-TEST_F(ExtensionTabUtilDelegateChromeOSTest, NoFilteringOutsidePublicSession) {
+TEST_F(ExtensionTabUtilDelegateChromeOSTest,
+       NoFilteringOutsidePublicSessionForWhitelisted) {
   ASSERT_FALSE(chromeos::LoginState::IsInitialized());
 
-  delegate_.ScrubTabForExtension(nullptr, nullptr, &tab_);
+  delegate_.ScrubTabForExtension(kWhitelistedId, &tab_);
+  EXPECT_EQ(kTestUrl, *tab_.url);
+}
+
+TEST_F(ExtensionTabUtilDelegateChromeOSTest,
+       NoFilteringOutsidePublicSessionForNonWhitelisted) {
+  ASSERT_FALSE(chromeos::LoginState::IsInitialized());
+
+  delegate_.ScrubTabForExtension(kNonWhitelistedId, &tab_);
+  EXPECT_EQ(kTestUrl, *tab_.url);
+}
+
+TEST_F(ExtensionTabUtilDelegateChromeOSTest,
+       NoFilteringInsidePublicSessionForWhitelisted) {
+  // Set Public Session state.
+  chromeos::LoginState::Initialize();
+  chromeos::LoginState::Get()->SetLoggedInState(
+    chromeos::LoginState::LOGGED_IN_ACTIVE,

[Devlin, 2017/05/03 15:14:15]

drive-by: indentation looks off here.

+    chromeos::LoginState::LOGGED_IN_USER_PUBLIC_ACCOUNT);
+
+  delegate_.ScrubTabForExtension(kWhitelistedId, &tab_);
   EXPECT_EQ(kTestUrl, *tab_.url);
+
+  // Reset state at the end of test.
+  chromeos::LoginState::Shutdown();
 }
 
-TEST_F(ExtensionTabUtilDelegateChromeOSTest, ScrubURL) {
+TEST_F(ExtensionTabUtilDelegateChromeOSTest,
+       FilterInsidePublicSessionNonWhitelisted) {
   // Set Public Session state.
   chromeos::LoginState::Initialize();

[Devlin, 2017/05/03 15:14:15]

This pattern:
chromeos::LoginState::Initialize();
chromeos::LoginState::Get()->SetLoggedInState(
    chromeos::LoginState::LOGGED_IN_ACTIVE,
    chromeos::LoginState::LOGGED_IN_USER_PUBLIC_ACCOUNT);

...

chromeos::LoginState::Shutdown();

is getting more common.  What would you think of adding a public test utility
class somewhere, like this:

// A class to start and shutdown public session state for a test.
class TestScopedLoginState {
 public:
  TestScopedLoginState() {
    chromeos::LoginState::Initialize();
    chromeos::LoginState::Get()->SetLoggedInState(
        chromeos::LoginState::LOGGED_IN_ACTIVE,
        chromeos::LoginState::LOGGED_IN_USER_PUBLIC_ACCOUNT);
  }
  ~TestScopedLoginState() {
    chromeos::LoginState::Shutdown();
  }
};

and then in these tests, we just do:
TEST_F(...) {
  TestScopedLoginState public_session_state;
  // Rest of the test.
  // No need to call shutdown at the end; the dtor of TestScopedLoginState
handles it.
}

(Other names might be better than TestScopedLoginState - ScopedPublicSession,
etc would also work.)

WDYT?

[Ivan Šandrk, 2017/05/03 17:52:46]

Yes, perfect! Done.

Logged a bug for myself to refactor other usages crbug.com/718078

   chromeos::LoginState::Get()->SetLoggedInState(

[Devlin, 2017/05/03 15:14:15]

ditto re indentation

     chromeos::LoginState::LOGGED_IN_ACTIVE,
     chromeos::LoginState::LOGGED_IN_USER_PUBLIC_ACCOUNT);
 
-  delegate_.ScrubTabForExtension(nullptr, nullptr, &tab_);
+  delegate_.ScrubTabForExtension(kNonWhitelistedId, &tab_);
   EXPECT_EQ(kFilteredUrl, *tab_.url);
 
   // Reset state at the end of test.