Index: components/password_manager/core/browser/affiliation_fetch_throttler_unittest.cc |
diff --git a/components/password_manager/core/browser/affiliation_fetch_throttler_unittest.cc b/components/password_manager/core/browser/affiliation_fetch_throttler_unittest.cc |
new file mode 100644 |
index 0000000000000000000000000000000000000000..39d333c3e6839ed768167dc76770ac1cd8717b8e |
--- /dev/null |
+++ b/components/password_manager/core/browser/affiliation_fetch_throttler_unittest.cc |
@@ -0,0 +1,231 @@ |
+// 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 "components/password_manager/core/browser/affiliation_fetch_throttler.h" |
+ |
+#include "base/callback.h" |
+#include "base/macros.h" |
+#include "base/memory/ref_counted.h" |
+#include "base/memory/scoped_ptr.h" |
+#include "base/message_loop/message_loop.h" |
+#include "base/run_loop.h" |
+#include "base/test/test_mock_time_task_runner.h" |
+#include "base/thread_task_runner_handle.h" |
+#include "base/time/tick_clock.h" |
+#include "base/time/time.h" |
+#include "components/password_manager/core/browser/affiliation_fetch_throttler_delegate.h" |
+#include "net/base/network_change_notifier.h" |
+#include "testing/gmock/include/gmock/gmock.h" |
+#include "testing/gtest/include/gtest/gtest.h" |
+ |
+namespace password_manager { |
+namespace { |
+ |
+class MockAffiliationFetchThrottlerDelegate |
+ : public testing::StrictMock<AffiliationFetchThrottlerDelegate> { |
+ public: |
+ MockAffiliationFetchThrottlerDelegate() {} |
+ |
+ MOCK_METHOD0(OnCanSendNetworkRequest, bool()); |
mmenke
2015/01/21 17:18:52
It's generally recommended you don't use GMOCK, to
mmenke
2015/01/21 17:18:53
In no tests does OnCanSendNetworkRequest return fa
engedy
2015/01/23 22:09:43
While the password_manager component uses GMock ex
engedy
2015/01/23 22:09:43
Done.
|
+ |
+ private: |
+ DISALLOW_COPY_AND_ASSIGN(MockAffiliationFetchThrottlerDelegate); |
+}; |
+ |
+class MockNetworkChangeNotifier : public net::NetworkChangeNotifier { |
+ public: |
+ MockNetworkChangeNotifier() : connection_type_(CONNECTION_UNKNOWN) {} |
+ |
+ void set_connection_type(ConnectionType connection_type) { |
+ connection_type_ = connection_type; |
+ NotifyObserversOfConnectionTypeChangeForTests(connection_type_); |
+ } |
+ |
+ ConnectionType GetCurrentConnectionType() const override { |
+ return connection_type_; |
+ } |
+ |
+ private: |
+ ConnectionType connection_type_; |
+}; |
+ |
+} // namespace |
+ |
+class AffiliationFetchThrottlerTest : public testing::Test { |
+ public: |
+ AffiliationFetchThrottlerTest() |
+ : network_change_notifier_(new MockNetworkChangeNotifier), |
mmenke
2015/01/21 17:18:53
Any reason this needs to be a scoped_ptr?
engedy
2015/01/23 22:09:43
Nope. Done.
|
+ task_runner_(new base::TestMockTimeTaskRunner) {} |
+ ~AffiliationFetchThrottlerTest() override {} |
+ |
+ void SetUp() override { |
+ policy_.reset(new AffiliationFetchThrottler( |
+ &mock_delegate_, task_runner_, task_runner_->GetMockTickClock())); |
mmenke
2015/01/21 17:18:52
Can this be done in the constructor, preferable in
engedy
2015/01/23 22:09:44
N/A. The instance under test is created in the tes
|
+ } |
+ |
+ void SimulateHasNetworkConnectivity(bool has_connectivity) { |
+ network_change_notifier_->set_connection_type( |
+ has_connectivity ? net::NetworkChangeNotifier::CONNECTION_UNKNOWN |
+ : net::NetworkChangeNotifier::CONNECTION_NONE); |
+ base::RunLoop().RunUntilIdle(); |
+ } |
+ |
+ // Forwards time, and given that MockAffiliationFetchThrottlerDelegate is a |
+ // strict mock, also ensures OnCanSendNetworkRequest() is not called back |
+ // during this period. |
+ void FastForwardTimeBySecs(double secs) { |
+ task_runner_->FastForwardBy(base::TimeDelta::FromSecondsD(secs)); |
+ } |
+ |
+ void FastForwardUntilNoTasksRemain() { |
+ task_runner_->FastForwardUntilNoTasksRemain(); |
+ } |
+ |
+ size_t GetPendingTaskCount() { return task_runner_->GetPendingTaskCount(); } |
mmenke
2015/01/21 17:18:52
const
engedy
2015/01/23 22:09:43
Done.
|
+ |
+ // Forwards time until the next OnCanSendNetworkRequest() callback, and |
+ // verifies that it occurs between |min_delay_sec| and |max_delay_sec| seconds |
+ // from now, inclusive. |
+ void AssertReleaseBetweenSecs(double min_delay_sec, double max_delay_sec) { |
+ base::TimeTicks ticks_at_start = task_runner_->GetCurrentMockTime(); |
+ base::TimeDelta min_delay = base::TimeDelta::FromSecondsD(min_delay_sec); |
+ if (min_delay.ToInternalValue() > 0) { |
+ task_runner_->FastForwardBy(min_delay - |
+ base::TimeDelta::FromInternalValue(1)); |
+ } |
+ EXPECT_CALL(mock_delegate_, OnCanSendNetworkRequest()) |
+ .Times(1) |
+ .WillRepeatedly(testing::Return(true)); |
+ task_runner_->FastForwardUntilNoTasksRemain(); |
+ ASSERT_TRUE(testing::Mock::VerifyAndClearExpectations(&mock_delegate_)); |
+ ASSERT_GE(base::TimeDelta::FromSecondsD(max_delay_sec), |
+ task_runner_->GetCurrentMockTime() - ticks_at_start); |
+ } |
+ |
+ AffiliationFetchThrottler& policy() { return *policy_.get(); } |
mmenke
2015/01/21 17:18:53
Seems like this should be returning an Affiliation
engedy
2015/01/23 22:09:44
Done.
|
+ |
+ private: |
+ // Needed because NetworkChangeNotifier uses ObserverList, which notifies |
+ // observers on the MessageLoop that belongs to the thread from which they |
+ // have registered. |
+ base::MessageLoop message_loop_; |
+ scoped_ptr<MockNetworkChangeNotifier> network_change_notifier_; |
+ scoped_refptr<base::TestMockTimeTaskRunner> task_runner_; |
+ MockAffiliationFetchThrottlerDelegate mock_delegate_; |
+ scoped_ptr<AffiliationFetchThrottler> policy_; |
mmenke
2015/01/21 17:18:53
Calling an "AffiliationFetchThrottler" policy_ is
engedy
2015/01/23 22:09:44
Done.
|
+ |
+ DISALLOW_COPY_AND_ASSIGN(AffiliationFetchThrottlerTest); |
+}; |
+ |
+TEST_F(AffiliationFetchThrottlerTest, SuccessfulRequests) { |
+ SimulateHasNetworkConnectivity(true); |
+ |
+ policy().SignalNetworkRequestNeeded(); |
+ AssertReleaseBetweenSecs(0, 0); |
+ |
+ // Signal while request is in flight should be ignored. |
mmenke
2015/01/21 17:18:53
We should make sure that it's ignored here. Is th
engedy
2015/01/23 22:09:44
Yes, the strict mock ensured this, but I have made
|
+ policy().SignalNetworkRequestNeeded(); |
+ FastForwardUntilNoTasksRemain(); |
+ policy().InformOfNetworkRequestComplete(true); |
+ FastForwardUntilNoTasksRemain(); |
+ |
+ // Duplicate the second signal 3 times: still only 1 callback should arrive. |
+ policy().SignalNetworkRequestNeeded(); |
+ policy().SignalNetworkRequestNeeded(); |
+ policy().SignalNetworkRequestNeeded(); |
+ AssertReleaseBetweenSecs(0, 0); |
+} |
+ |
+TEST_F(AffiliationFetchThrottlerTest, FailedRequests) { |
+ SimulateHasNetworkConnectivity(true); |
+ |
+ policy().SignalNetworkRequestNeeded(); |
+ ASSERT_NO_FATAL_FAILURE(AssertReleaseBetweenSecs(0, 0)); |
+ policy().InformOfNetworkRequestComplete(false); |
+ |
+ // Request after first failure should be delayed by 10 seconds with 50% |
+ // fuzzing factor. |
+ policy().SignalNetworkRequestNeeded(); |
+ ASSERT_NO_FATAL_FAILURE(AssertReleaseBetweenSecs(5, 10)); |
mmenke
2015/01/21 17:18:53
Relying on exact values for doubles just seems lik
engedy
2015/01/23 22:09:44
Not that we are not relying on *exact* value: this
mmenke
2015/01/23 22:39:53
If we get a 5 or a 10 in the random calculation (A
engedy
2015/01/24 00:35:38
Not sure I understand what you mean by this. Excep
|
+ policy().InformOfNetworkRequestComplete(false); |
+ |
+ // Request after first failure should be delayed by 40 seconds with 50% |
+ // fuzzing factor, applied twice. |
mmenke
2015/01/21 17:18:52
This isn't true. Jitter factor is applied indepen
engedy
2015/01/23 22:09:44
Right. Done.
|
+ policy().SignalNetworkRequestNeeded(); |
+ ASSERT_NO_FATAL_FAILURE(AssertReleaseBetweenSecs(10, 40)); |
+ policy().InformOfNetworkRequestComplete(false); |
+} |
+ |
+TEST_F(AffiliationFetchThrottlerTest, GracePeriodAfterConnectivityIsRestored) { |
+ SimulateHasNetworkConnectivity(false); |
+ |
+ // After connectivity is re-established, the first request should be delayed |
+ // by 10 seconds with 50% fuzzing factor. |
+ policy().SignalNetworkRequestNeeded(); |
mmenke
2015/01/21 17:18:52
FastForwardUntilNoTasksRemain();? (And same goes
engedy
2015/01/23 22:09:43
I have added it here, and verified elsewhere, plea
|
+ SimulateHasNetworkConnectivity(true); |
+ ASSERT_NO_FATAL_FAILURE(AssertReleaseBetweenSecs(5, 10)); |
mmenke
2015/01/21 17:18:53
These numbers should not be hardcoded. Should use
engedy
2015/01/23 22:09:44
Done.
|
+ policy().InformOfNetworkRequestComplete(true); |
+ |
+ // The next request should not be delayed. |
+ policy().SignalNetworkRequestNeeded(); |
+ AssertReleaseBetweenSecs(0, 0); |
+} |
+ |
+// Same as GracePeriodAfterConnectivityIsRestored, but the network comes back |
+// just before SignalNetworkRequestNeeded() is called. |
+TEST_F(AffiliationFetchThrottlerTest, GracePeriodAfterConnectivityIsRestored2) { |
+ SimulateHasNetworkConnectivity(false); |
+ |
+ SimulateHasNetworkConnectivity(true); |
+ policy().SignalNetworkRequestNeeded(); |
+ ASSERT_NO_FATAL_FAILURE(AssertReleaseBetweenSecs(5, 10)); |
mmenke
2015/01/21 17:18:53
You may want to add a way to EXPECT on the actual
engedy
2015/01/23 22:09:44
Done.
|
+ policy().InformOfNetworkRequestComplete(true); |
+ |
+ policy().SignalNetworkRequestNeeded(); |
+ AssertReleaseBetweenSecs(0, 0); |
+} |
+ |
+// TODO(engedy): This test is flaky. Investigate. |
+TEST_F(AffiliationFetchThrottlerTest, ConnectivityLostDuringBackoff) { |
+ SimulateHasNetworkConnectivity(true); |
+ |
+ policy().SignalNetworkRequestNeeded(); |
+ ASSERT_NO_FATAL_FAILURE(AssertReleaseBetweenSecs(0, 0)); |
+ policy().InformOfNetworkRequestComplete(false); |
+ |
+ policy().SignalNetworkRequestNeeded(); |
+ SimulateHasNetworkConnectivity(false); |
+ |
+ // Let the exponential backoff delay expire, and verify nothing happens. |
+ FastForwardUntilNoTasksRemain(); |
+ |
+ // Verify that the request is, however, sent after 10 seconds with 50% fuzzing |
+ // factor. |
+ SimulateHasNetworkConnectivity(true); |
+ ASSERT_NO_FATAL_FAILURE(AssertReleaseBetweenSecs(5, 10)); |
+} |
+ |
+TEST_F(AffiliationFetchThrottlerTest, |
+ ConnectivityLostAndRestoredDuringBackoff) { |
+ SimulateHasNetworkConnectivity(true); |
+ |
+ policy().SignalNetworkRequestNeeded(); |
+ ASSERT_NO_FATAL_FAILURE(AssertReleaseBetweenSecs(0, 0)); |
+ policy().InformOfNetworkRequestComplete(false); |
+ |
+ policy().SignalNetworkRequestNeeded(); |
+ |
+ // Also verify that a flaky connection will not flood the task queue with lots |
+ // of tasks. |
mmenke
2015/01/21 17:18:52
I don't think this is needed... The NCN itself al
engedy
2015/01/23 22:09:43
This mostly test what you have pointed out earlier
|
+ for (size_t t = 1; t <= 5; ++t) { |
+ SimulateHasNetworkConnectivity(false); |
+ SimulateHasNetworkConnectivity(true); |
+ FastForwardTimeBySecs(1); |
+ } |
+ EXPECT_EQ(1u, GetPendingTaskCount()); |
+ |
+ ASSERT_NO_FATAL_FAILURE(AssertReleaseBetweenSecs(5, 10)); |
+} |
mmenke
2015/01/21 17:18:53
Do we have a test where connectivity is lost and r
engedy
2015/01/23 22:09:44
I have added two, though I wasn't sure if the URLF
mmenke
2015/01/23 22:39:53
You shouldn't depend on what it does - it depends
engedy
2015/01/26 19:39:34
I have added an additional test.
|
+ |
+} // namespace password_manager |