OLD | NEW |
---|---|
(Empty) | |
1 // Copyright 2015 The Chromium Authors. All rights reserved. | |
2 // Use of this source code is governed by a BSD-style license that can be | |
3 // found in the LICENSE file. | |
4 | |
5 #include "components/password_manager/core/browser/affiliation_fetch_throttler.h " | |
6 | |
7 #include "base/callback.h" | |
8 #include "base/macros.h" | |
9 #include "base/memory/ref_counted.h" | |
10 #include "base/memory/scoped_ptr.h" | |
11 #include "base/message_loop/message_loop.h" | |
12 #include "base/run_loop.h" | |
13 #include "base/test/test_mock_time_task_runner.h" | |
14 #include "base/thread_task_runner_handle.h" | |
15 #include "base/time/tick_clock.h" | |
16 #include "base/time/time.h" | |
17 #include "components/password_manager/core/browser/affiliation_fetch_throttler_d elegate.h" | |
18 #include "net/base/network_change_notifier.h" | |
19 #include "testing/gmock/include/gmock/gmock.h" | |
20 #include "testing/gtest/include/gtest/gtest.h" | |
21 | |
22 namespace password_manager { | |
23 namespace { | |
24 | |
25 class MockAffiliationFetchThrottlerDelegate | |
26 : public testing::StrictMock<AffiliationFetchThrottlerDelegate> { | |
27 public: | |
28 MockAffiliationFetchThrottlerDelegate() {} | |
29 | |
30 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.
| |
31 | |
32 private: | |
33 DISALLOW_COPY_AND_ASSIGN(MockAffiliationFetchThrottlerDelegate); | |
34 }; | |
35 | |
36 class MockNetworkChangeNotifier : public net::NetworkChangeNotifier { | |
37 public: | |
38 MockNetworkChangeNotifier() : connection_type_(CONNECTION_UNKNOWN) {} | |
39 | |
40 void set_connection_type(ConnectionType connection_type) { | |
41 connection_type_ = connection_type; | |
42 NotifyObserversOfConnectionTypeChangeForTests(connection_type_); | |
43 } | |
44 | |
45 ConnectionType GetCurrentConnectionType() const override { | |
46 return connection_type_; | |
47 } | |
48 | |
49 private: | |
50 ConnectionType connection_type_; | |
51 }; | |
52 | |
53 } // namespace | |
54 | |
55 class AffiliationFetchThrottlerTest : public testing::Test { | |
56 public: | |
57 AffiliationFetchThrottlerTest() | |
58 : 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.
| |
59 task_runner_(new base::TestMockTimeTaskRunner) {} | |
60 ~AffiliationFetchThrottlerTest() override {} | |
61 | |
62 void SetUp() override { | |
63 policy_.reset(new AffiliationFetchThrottler( | |
64 &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
| |
65 } | |
66 | |
67 void SimulateHasNetworkConnectivity(bool has_connectivity) { | |
68 network_change_notifier_->set_connection_type( | |
69 has_connectivity ? net::NetworkChangeNotifier::CONNECTION_UNKNOWN | |
70 : net::NetworkChangeNotifier::CONNECTION_NONE); | |
71 base::RunLoop().RunUntilIdle(); | |
72 } | |
73 | |
74 // Forwards time, and given that MockAffiliationFetchThrottlerDelegate is a | |
75 // strict mock, also ensures OnCanSendNetworkRequest() is not called back | |
76 // during this period. | |
77 void FastForwardTimeBySecs(double secs) { | |
78 task_runner_->FastForwardBy(base::TimeDelta::FromSecondsD(secs)); | |
79 } | |
80 | |
81 void FastForwardUntilNoTasksRemain() { | |
82 task_runner_->FastForwardUntilNoTasksRemain(); | |
83 } | |
84 | |
85 size_t GetPendingTaskCount() { return task_runner_->GetPendingTaskCount(); } | |
mmenke
2015/01/21 17:18:52
const
engedy
2015/01/23 22:09:43
Done.
| |
86 | |
87 // Forwards time until the next OnCanSendNetworkRequest() callback, and | |
88 // verifies that it occurs between |min_delay_sec| and |max_delay_sec| seconds | |
89 // from now, inclusive. | |
90 void AssertReleaseBetweenSecs(double min_delay_sec, double max_delay_sec) { | |
91 base::TimeTicks ticks_at_start = task_runner_->GetCurrentMockTime(); | |
92 base::TimeDelta min_delay = base::TimeDelta::FromSecondsD(min_delay_sec); | |
93 if (min_delay.ToInternalValue() > 0) { | |
94 task_runner_->FastForwardBy(min_delay - | |
95 base::TimeDelta::FromInternalValue(1)); | |
96 } | |
97 EXPECT_CALL(mock_delegate_, OnCanSendNetworkRequest()) | |
98 .Times(1) | |
99 .WillRepeatedly(testing::Return(true)); | |
100 task_runner_->FastForwardUntilNoTasksRemain(); | |
101 ASSERT_TRUE(testing::Mock::VerifyAndClearExpectations(&mock_delegate_)); | |
102 ASSERT_GE(base::TimeDelta::FromSecondsD(max_delay_sec), | |
103 task_runner_->GetCurrentMockTime() - ticks_at_start); | |
104 } | |
105 | |
106 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.
| |
107 | |
108 private: | |
109 // Needed because NetworkChangeNotifier uses ObserverList, which notifies | |
110 // observers on the MessageLoop that belongs to the thread from which they | |
111 // have registered. | |
112 base::MessageLoop message_loop_; | |
113 scoped_ptr<MockNetworkChangeNotifier> network_change_notifier_; | |
114 scoped_refptr<base::TestMockTimeTaskRunner> task_runner_; | |
115 MockAffiliationFetchThrottlerDelegate mock_delegate_; | |
116 scoped_ptr<AffiliationFetchThrottler> policy_; | |
mmenke
2015/01/21 17:18:53
Calling an "AffiliationFetchThrottler" policy_ is
engedy
2015/01/23 22:09:44
Done.
| |
117 | |
118 DISALLOW_COPY_AND_ASSIGN(AffiliationFetchThrottlerTest); | |
119 }; | |
120 | |
121 TEST_F(AffiliationFetchThrottlerTest, SuccessfulRequests) { | |
122 SimulateHasNetworkConnectivity(true); | |
123 | |
124 policy().SignalNetworkRequestNeeded(); | |
125 AssertReleaseBetweenSecs(0, 0); | |
126 | |
127 // 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
| |
128 policy().SignalNetworkRequestNeeded(); | |
129 FastForwardUntilNoTasksRemain(); | |
130 policy().InformOfNetworkRequestComplete(true); | |
131 FastForwardUntilNoTasksRemain(); | |
132 | |
133 // Duplicate the second signal 3 times: still only 1 callback should arrive. | |
134 policy().SignalNetworkRequestNeeded(); | |
135 policy().SignalNetworkRequestNeeded(); | |
136 policy().SignalNetworkRequestNeeded(); | |
137 AssertReleaseBetweenSecs(0, 0); | |
138 } | |
139 | |
140 TEST_F(AffiliationFetchThrottlerTest, FailedRequests) { | |
141 SimulateHasNetworkConnectivity(true); | |
142 | |
143 policy().SignalNetworkRequestNeeded(); | |
144 ASSERT_NO_FATAL_FAILURE(AssertReleaseBetweenSecs(0, 0)); | |
145 policy().InformOfNetworkRequestComplete(false); | |
146 | |
147 // Request after first failure should be delayed by 10 seconds with 50% | |
148 // fuzzing factor. | |
149 policy().SignalNetworkRequestNeeded(); | |
150 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
| |
151 policy().InformOfNetworkRequestComplete(false); | |
152 | |
153 // Request after first failure should be delayed by 40 seconds with 50% | |
154 // 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.
| |
155 policy().SignalNetworkRequestNeeded(); | |
156 ASSERT_NO_FATAL_FAILURE(AssertReleaseBetweenSecs(10, 40)); | |
157 policy().InformOfNetworkRequestComplete(false); | |
158 } | |
159 | |
160 TEST_F(AffiliationFetchThrottlerTest, GracePeriodAfterConnectivityIsRestored) { | |
161 SimulateHasNetworkConnectivity(false); | |
162 | |
163 // After connectivity is re-established, the first request should be delayed | |
164 // by 10 seconds with 50% fuzzing factor. | |
165 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
| |
166 SimulateHasNetworkConnectivity(true); | |
167 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.
| |
168 policy().InformOfNetworkRequestComplete(true); | |
169 | |
170 // The next request should not be delayed. | |
171 policy().SignalNetworkRequestNeeded(); | |
172 AssertReleaseBetweenSecs(0, 0); | |
173 } | |
174 | |
175 // Same as GracePeriodAfterConnectivityIsRestored, but the network comes back | |
176 // just before SignalNetworkRequestNeeded() is called. | |
177 TEST_F(AffiliationFetchThrottlerTest, GracePeriodAfterConnectivityIsRestored2) { | |
178 SimulateHasNetworkConnectivity(false); | |
179 | |
180 SimulateHasNetworkConnectivity(true); | |
181 policy().SignalNetworkRequestNeeded(); | |
182 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.
| |
183 policy().InformOfNetworkRequestComplete(true); | |
184 | |
185 policy().SignalNetworkRequestNeeded(); | |
186 AssertReleaseBetweenSecs(0, 0); | |
187 } | |
188 | |
189 // TODO(engedy): This test is flaky. Investigate. | |
190 TEST_F(AffiliationFetchThrottlerTest, ConnectivityLostDuringBackoff) { | |
191 SimulateHasNetworkConnectivity(true); | |
192 | |
193 policy().SignalNetworkRequestNeeded(); | |
194 ASSERT_NO_FATAL_FAILURE(AssertReleaseBetweenSecs(0, 0)); | |
195 policy().InformOfNetworkRequestComplete(false); | |
196 | |
197 policy().SignalNetworkRequestNeeded(); | |
198 SimulateHasNetworkConnectivity(false); | |
199 | |
200 // Let the exponential backoff delay expire, and verify nothing happens. | |
201 FastForwardUntilNoTasksRemain(); | |
202 | |
203 // Verify that the request is, however, sent after 10 seconds with 50% fuzzing | |
204 // factor. | |
205 SimulateHasNetworkConnectivity(true); | |
206 ASSERT_NO_FATAL_FAILURE(AssertReleaseBetweenSecs(5, 10)); | |
207 } | |
208 | |
209 TEST_F(AffiliationFetchThrottlerTest, | |
210 ConnectivityLostAndRestoredDuringBackoff) { | |
211 SimulateHasNetworkConnectivity(true); | |
212 | |
213 policy().SignalNetworkRequestNeeded(); | |
214 ASSERT_NO_FATAL_FAILURE(AssertReleaseBetweenSecs(0, 0)); | |
215 policy().InformOfNetworkRequestComplete(false); | |
216 | |
217 policy().SignalNetworkRequestNeeded(); | |
218 | |
219 // Also verify that a flaky connection will not flood the task queue with lots | |
220 // 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
| |
221 for (size_t t = 1; t <= 5; ++t) { | |
222 SimulateHasNetworkConnectivity(false); | |
223 SimulateHasNetworkConnectivity(true); | |
224 FastForwardTimeBySecs(1); | |
225 } | |
226 EXPECT_EQ(1u, GetPendingTaskCount()); | |
227 | |
228 ASSERT_NO_FATAL_FAILURE(AssertReleaseBetweenSecs(5, 10)); | |
229 } | |
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.
| |
230 | |
231 } // namespace password_manager | |
OLD | NEW |