Handle origin-specific password deletion in BrowsingDataRemover

chrome/browser/browsing_data/browsing_data_remover_unittest.cc

 // Copyright (c) 2012 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/browsing_data/browsing_data_remover.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <set>
@@ skipping 860 matching lines @@
 
   content::MockDownloadManager* download_manager() { return download_manager_; }
 
  private:
   content::MockDownloadManager* download_manager_;
   ChromeDownloadManagerDelegate chrome_download_manager_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(RemoveDownloadsTester);
 };
 
+class RemovePasswordsTester {
+ public:
+  explicit RemovePasswordsTester(TestingProfile* testing_profile) {
+    PasswordStoreFactory::GetInstance()->SetTestingFactoryAndUse(
+        testing_profile,
+        password_manager::BuildPasswordStore<
+            content::BrowserContext,
+            testing::NiceMock<password_manager::MockPasswordStore>>);

[Timo Reimann, 2016/02/01 10:07:20]

Note that I additionally wrapped the mocked password store into a NiceMock here
in order to prevent emitting warnings for NotifyLoginsChanged being called for
the new test. While this is specific to the new test, the Google Mock
documentation recommends to use NiceMocks by default[1]. Thus, it seemed fine to
me to put it into the general password helper class.

[1]
https://github.com/google/googlemock/blob/master/googlemock/docs/CookBook.md#...
(last paragraph)

+
+    store_ = static_cast<password_manager::MockPasswordStore*>(
+        PasswordStoreFactory::GetInstance()
+            ->GetForProfile(testing_profile, ServiceAccessType::EXPLICIT_ACCESS)
+            .get());
+  }
+
+  password_manager::MockPasswordStore* store() { return store_; }
+
+ private:
+  password_manager::MockPasswordStore* store_;
+
+  DISALLOW_COPY_AND_ASSIGN(RemovePasswordsTester);
+};
+
 // Test Class ----------------------------------------------------------------
 
 class BrowsingDataRemoverTest : public testing::Test {
  public:
   BrowsingDataRemoverTest()
       : profile_(new TestingProfile()),
         clear_domain_reliability_tester_(GetProfile()) {
     callback_subscription_ =
         BrowsingDataRemover::RegisterOnBrowsingDataRemovedCallback(
             base::Bind(&BrowsingDataRemoverTest::NotifyWithDetails,
@@ skipping 1259 matching lines @@
   const url::Origin expectedOrigin(kOrigin1);
 
   EXPECT_CALL(*tester.download_manager(),
               RemoveDownloadsByOriginAndTime(SameOrigin(expectedOrigin), _, _));
 
   BlockUntilOriginDataRemoved(BrowsingDataRemover::EVERYTHING,
                               BrowsingDataRemover::REMOVE_DOWNLOADS, kOrigin1);
 }
 
 TEST_F(BrowsingDataRemoverTest, RemovePasswordStatistics) {
-  PasswordStoreFactory::GetInstance()->SetTestingFactoryAndUse(
-      GetProfile(),
-      password_manager::BuildPasswordStore<
-          content::BrowserContext, password_manager::MockPasswordStore>);
-  password_manager::MockPasswordStore* store =
-      static_cast<password_manager::MockPasswordStore*>(
-          PasswordStoreFactory::GetInstance()
-              ->GetForProfile(GetProfile(), ServiceAccessType::EXPLICIT_ACCESS)
-              .get());
-  EXPECT_CALL(*store, RemoveStatisticsCreatedBetweenImpl(base::Time(),
-                                                         base::Time::Max()));
+  RemovePasswordsTester tester(GetProfile());
+
+  EXPECT_CALL(*tester.store(), RemoveStatisticsCreatedBetweenImpl(
+                                   base::Time(), base::Time::Max()));
   BlockUntilBrowsingDataRemoved(
       BrowsingDataRemover::EVERYTHING,
       BrowsingDataRemover::REMOVE_HISTORY, false);
 }
+
+TEST_F(BrowsingDataRemoverTest, RemoveSameOriginPasswords) {
+  RemovePasswordsTester tester(GetProfile());
+  const url::Origin expectedOrigin(kOrigin1);
+
+  EXPECT_CALL(*tester.store(),
+              RemoveLoginsByOriginAndTimeImpl(SameOrigin(expectedOrigin), _, _))
+      .WillOnce(Return(password_manager::PasswordStoreChangeList()));
+  BlockUntilOriginDataRemoved(BrowsingDataRemover::EVERYTHING,
+                              BrowsingDataRemover::REMOVE_PASSWORDS, kOrigin1);

[Mike West, 2016/02/04 18:25:11]

Can you add a test that passes in a URL that produces a unique origin?
`data:text/html,...` for instance shouldn't actually remove any data from the
system.

[Timo Reimann, 2016/02/05 01:12:38]

(Assuming you meant that it shouldn't remove any *origin-specific* data, I went
ahead and:)

Added tests for both the previously existing download case and the password one.
I'm passing in GURL() to account for the switch to checking for emptiness.

+}