Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2603583002:
Add user-gesture-related tests for ContentAutofillDriver (Closed)
|
Created:
3 years, 12 months ago by vabr (Chromium) Modified:
3 years, 12 months ago Reviewers:
dvadym CC:
chromium-reviews, rouslan+autofill_chromium.org, jam, sebsg+autofillwatch_chromium.org, browser-components-watch_chromium.org, mathp+autofillwatch_chromium.org, darin-cc_chromium.org, vabr+watchlistautofill_chromium.org, estade+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd user-gesture-related tests for ContentAutofillDriver
The methods ContentAutofillDriver::FirstUserGestureObserved and
ContentAutofillDriver::NotifyFirstUserGestureObservedInTab do not seem to be
tested yet. This CL adds tests for that.
This is in preparation to fixing https://crbug.com/669045, to avoid inflating
the CL with the fix by adding tests for existing code.
BUG=669045
Committed: https://crrev.com/603bbb6727a01a230ec804a2367537dd9aa11dc5
Cr-Commit-Position: refs/heads/master@{#440624}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Simpler test #Messages
Total messages: 17 (10 generated)
The CQ bit was checked by vabr@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
vabr@chromium.org changed reviewers: + dvadym@chromium.org
Hi Vadym, Could you please take a look? Cheers, Vaclav
https://codereview.chromium.org/2603583002/diff/1/components/autofill/content... File components/autofill/content/browser/content_autofill_driver_unittest.cc (right): https://codereview.chromium.org/2603583002/diff/1/components/autofill/content... components/autofill/content/browser/content_autofill_driver_unittest.cc:490: fake_agent_.SetQuitLoopClosure(run_loop.QuitClosure()); I just realised that this makes no sense when FirstUserGestureObservedInTab is just a mocked method, not calling CallDone() in the fake agent. I don't think QuitClosure is needed here to make RunUntilIdle() return, because there are no repeated tasks such as animated web pages run. To be honest, I believe this is also not needed for the rest of the tests, but I won't touch that in this CL. Perhaps in a follow-up.
The CQ bit was checked by vabr@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Test fixed, sorry for the interruption. This is fit for review again. Cheers, Vaclav
On 2016/12/23 13:00:13, vabr (Chromium) wrote: > Test fixed, sorry for the interruption. This is fit for review again. > > Cheers, > Vaclav LGTM
The CQ bit was unchecked by vabr@chromium.org
The CQ bit was checked by vabr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1482498785409430,
"parent_rev": "5f53f04fed530390a4b6d6298b84d05a731ffc56", "commit_rev":
"e713efefeb799e7cb6e04768e970aa89838f8ad8"}
Message was sent while issue was closed.
Description was changed from ========== Add user-gesture-related tests for ContentAutofillDriver The methods ContentAutofillDriver::FirstUserGestureObserved and ContentAutofillDriver::NotifyFirstUserGestureObservedInTab do not seem to be tested yet. This CL adds tests for that. This is in preparation to fixing https://crbug.com/669045, to avoid inflating the CL with the fix by adding tests for existing code. BUG=669045 ========== to ========== Add user-gesture-related tests for ContentAutofillDriver The methods ContentAutofillDriver::FirstUserGestureObserved and ContentAutofillDriver::NotifyFirstUserGestureObservedInTab do not seem to be tested yet. This CL adds tests for that. This is in preparation to fixing https://crbug.com/669045, to avoid inflating the CL with the fix by adding tests for existing code. BUG=669045 Review-Url: https://codereview.chromium.org/2603583002 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Add user-gesture-related tests for ContentAutofillDriver The methods ContentAutofillDriver::FirstUserGestureObserved and ContentAutofillDriver::NotifyFirstUserGestureObservedInTab do not seem to be tested yet. This CL adds tests for that. This is in preparation to fixing https://crbug.com/669045, to avoid inflating the CL with the fix by adding tests for existing code. BUG=669045 Review-Url: https://codereview.chromium.org/2603583002 ========== to ========== Add user-gesture-related tests for ContentAutofillDriver The methods ContentAutofillDriver::FirstUserGestureObserved and ContentAutofillDriver::NotifyFirstUserGestureObservedInTab do not seem to be tested yet. This CL adds tests for that. This is in preparation to fixing https://crbug.com/669045, to avoid inflating the CL with the fix by adding tests for existing code. BUG=669045 Committed: https://crrev.com/603bbb6727a01a230ec804a2367537dd9aa11dc5 Cr-Commit-Position: refs/heads/master@{#440624} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/603bbb6727a01a230ec804a2367537dd9aa11dc5 Cr-Commit-Position: refs/heads/master@{#440624} |
