PS - Filtering activeTab URL

chrome/browser/chromeos/extensions/extension_tab_util_delegate_chromeos_unittest.cc

 // Copyright 2017 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/chromeos/extensions/extension_tab_util_delegate_chromeos.h"
 
 #include <string>
 
 #include "chromeos/login/login_state.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 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/";
 
 }  // namespace
 
 class ExtensionTabUtilDelegateChromeOSTest : public testing::Test {
  protected:
   void SetUp() override;
 
   ExtensionTabUtilDelegateChromeOS delegate_;
   api::tabs::Tab tab_;
 };
 
 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, ScrubURL) {
+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(nullptr, nullptr, &tab_);
+  delegate_.ScrubTabForExtension(kWhitelistedId, &tab_);
+  EXPECT_EQ(kTestUrl, *tab_.url);
+
+  // Reset state at the end of test.
+  chromeos::LoginState::Shutdown();
+}
+
+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(kNonWhitelistedId, &tab_);
   EXPECT_EQ(kFilteredUrl, *tab_.url);
 
   // Reset state at the end of test.
   chromeos::LoginState::Shutdown();
 }
 
 }  // namespace extensions