Implement an auto-advance-off mode for NavigationSimulator |
||
Issue descriptionSophisticated NavigationThrottle tests may want to use another task runner for async tests other than the one the main thread runs on. Right now, this is not possible since the NavigationThrottle internally spins a RunLoop, which blocks the test from making progress on any other task runners. Since most NavigationSimulator consumers prefer the auto-advance functionality, this could be an optional parameter SetAutoAdvance(bool). If false, the client can call Wait() on the simulator to manually advance after they've done the processing they need.
,
Mar 22 2018
Here is a simple example test we could write with the new system. In general this approach avoids a callback-driven design (like what was implemented before, an OnDeferred callback), in favor of simpler linear tests. The ability to use e.g. mock time task runners is very powerful. This test just does something silly but you could start testing things like cancellation or timeouts using mock time. // Defers a navigation in some cases, via some async work on another task runner. class DelayingThrottle; auto task_runner = base::MakeRefCounted<TestMockTimeTaskRunner>(); TestNavigationThrottleInserter inserter(base::BindOnce(&CreateThrottle, task_runner)); auto simulator = NavigationSimulator::CreateRendererInitiated(url, main_rfh()); simulator->SetAutoAdvance(false); simulator->Start(); EXPECT_TRUE(simulator->IsDeferred()); // Calling Wait() here would hang, since we need to fast forward time manually // on the mock time task runner, which should triggle the throttle to post a // task to resume the navigation. task_runner->FastForwardBy(kDelayMs); simulator->Wait(); simulator->ReadyToCommit(); EXPECT_FALSE(simulator->IsDeferred()); simulator->Commit();
,
Mar 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a4fdbbaf8b99ba70b29c1f9192ba598c2e1c58f8 commit a4fdbbaf8b99ba70b29c1f9192ba598c2e1c58f8 Author: Charlie Harrison <csharrison@chromium.org> Date: Tue Mar 27 18:44:04 2018 NavigationSimulator: Refactor of WaitForThrottleChecksComplete The end goal is to get NavigationSimulator to a state where we can call: simulator->SetAutoAdvance(false); simulator->Start(); And have the simulator *not* spin the run loop in the background. This will help in writing more sophisticated NavigationThrottle tests which use multiple task runners to execute async tasks. This particular CL does a few things: 1. Removes base::RunLoop().RunUntilIdle call. This is not great practice to have in test infra, since it can cause strange interactions with tests and they may start relying on unintentional behavior. 2. Collects various after-throttle checks into closures, to be called when the throttle check complete closure is called (rather than relying on base::RunLoop to keep everything on the stack). This isn't so useful now, but it will if we stop *always* waiting. 3. Slightly modifies checks in various places that relied on some of the behavior that we've changed. Two things changed here: a. "Complete" code internal to the NavigationThrottle now runs directly after throttle checks are complete (when complete_callback_for_testing_ is called on the nav handle), rather than when the RunLoop quits. This caused us to remove one check checking that we've finished the navigation. b. Because we no longer RunUntilIdle() in WaitForThrottleChecksComplete, code that relies on the nav finishing after the post-task in NavigationRequest::OnStartChecksComplete will need to either wait or spin the run loop to see the end of the navigation. This is a pretty small edge case and it is pretty easy to either write a helper to make this explicit or use a WCO. The only test impacted by this change is a NavigationSimulator test, and a test which incidentally relied on this behavior for unrelated reasons. Bug: 822275 Change-Id: Ice6b215fa285a6a944fdf85108303515d109be77 Reviewed-on: https://chromium-review.googlesource.com/961819 Reviewed-by: oysteine <oysteine@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Commit-Queue: Charlie Harrison <csharrison@chromium.org> Cr-Commit-Position: refs/heads/master@{#546184} [modify] https://crrev.com/a4fdbbaf8b99ba70b29c1f9192ba598c2e1c58f8/chrome/browser/resource_coordinator/tab_activity_watcher_unittest.cc [modify] https://crrev.com/a4fdbbaf8b99ba70b29c1f9192ba598c2e1c58f8/components/subresource_filter/content/browser/activation_state_computing_navigation_throttle_unittest.cc [modify] https://crrev.com/a4fdbbaf8b99ba70b29c1f9192ba598c2e1c58f8/content/public/test/navigation_simulator.cc [modify] https://crrev.com/a4fdbbaf8b99ba70b29c1f9192ba598c2e1c58f8/content/public/test/navigation_simulator.h [modify] https://crrev.com/a4fdbbaf8b99ba70b29c1f9192ba598c2e1c58f8/content/test/navigation_simulator_unittest.cc
,
Mar 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5cc740dc16144680a94bcba70721d837364343e4 commit 5cc740dc16144680a94bcba70721d837364343e4 Author: Charlie Harrison <csharrison@chromium.org> Date: Tue Mar 27 20:52:46 2018 NavigationSimulator: Implement Wait() and SetAutoAdvance(false) This CL augments the NavigationSimulator to allow tests to drive it manually through its stages. This is implemented via a method SetAutoAdvance(bool). The CL also implements a Wait() method which spins the run loop waiting for throttle checks to complete, and an IsDeferred() method which returns whether some throttle is currently deferring the navigation. This is used to implement one test outside of //content. Bug: 822275 Change-Id: I394e33cf89ee298c92d2c6554ca0c591e080a695 Reviewed-on: https://chromium-review.googlesource.com/964389 Commit-Queue: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/heads/master@{#546248} [modify] https://crrev.com/5cc740dc16144680a94bcba70721d837364343e4/components/subresource_filter/content/browser/activation_state_computing_navigation_throttle_unittest.cc [modify] https://crrev.com/5cc740dc16144680a94bcba70721d837364343e4/content/public/test/navigation_simulator.cc [modify] https://crrev.com/5cc740dc16144680a94bcba70721d837364343e4/content/public/test/navigation_simulator.h [modify] https://crrev.com/5cc740dc16144680a94bcba70721d837364343e4/content/test/navigation_simulator_unittest.cc
,
Mar 27 2018
|
||
►
Sign in to add a comment |
||
Comment 1 by csharrison@chromium.org
, Mar 20 2018