Integration of PasswordReuseDetector into PasswordStore.

chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc

 // Copyright 2014 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/passwords/manage_passwords_bubble_model.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/memory/ptr_util.h"
@@ skipping 92 matching lines @@
 
   void SetUp() override {
     test_web_contents_.reset(
         content::WebContentsTester::CreateTestWebContents(&profile_, nullptr));
     mock_delegate_.reset(new testing::StrictMock<PasswordsModelDelegateMock>);
     PasswordStoreFactory::GetInstance()->SetTestingFactoryAndUse(
         profile(),
         password_manager::BuildPasswordStore<
             content::BrowserContext,
             testing::StrictMock<password_manager::MockPasswordStore>>);
+    EXPECT_CALL(*GetStore(), FillAutofillableLogins(_));

[vabr (Chromium), 2016/12/22 16:15:12]

It is a bit strange to expect a call and then return the function. Normally the
code which will result in the expected call should be below the EXPECT macro.
What is causing the call to FillAutofillableLogins? Could you put the
EXPECT_CALL before that?

[dvadym, 2016/12/22 16:23:12]

PasswordStore initializes PasswordReuseDetector, and it causes call of
FillAutofillableLogins. So a call of BuildPasswordStore itself causes a call of
FillAutofillableLogins, but since it's on a posted task it happens after
returning from BuildPasswordStore. 
Note that there is no thread race, since unit tests are running on 1 thread and
any posted task is always running after current function execution is finished.
I can explain all this in comments? Would it be ok?

[vabr (Chromium), 2016/12/22 16:33:30]

Thanks for the explanation. In that case please move the EXPECT_CALL before line
108. You might comment on the expectation being fulfilled in a task posted as a
result of the call below.

[dvadym, 2016/12/22 16:57:57]

If I correctly understand, line 108 creates password store, so I can't use
EXPECT_CALL on store object before it.

[vabr (Chromium), 2016/12/22 17:14:31]

You are right, sorry for missing that!
Then please do comment that the expectation is met in a task posted by the above
construction.

[dvadym, 2016/12/22 17:31:18]

Thanks, I've updated comment.

   }
 
   void TearDown() override {
     // Reset the delegate first. It can happen if the user closes the tab.
     mock_delegate_.reset();
     model_.reset();
     variations::testing::ClearAllVariationIDs();
     variations::testing::ClearAllVariationParams();
   }
 
@@ skipping 460 matching lines @@
      ManageLinkTarget::SETTINGS_PAGE},
     {nullptr, SmartLockStatus::DISABLE, ManageLinkTarget::SETTINGS_PAGE},
     {"Default", SmartLockStatus::DISABLE, ManageLinkTarget::SETTINGS_PAGE},
 };
 
 }  // namespace
 
 INSTANTIATE_TEST_CASE_P(Default,
                         ManagePasswordsBubbleModelManageLinkTest,
                         ::testing::ValuesIn(kManageLinkTestCases));