• Charlie Harrison's avatar
    NavigationSimulator: Refactor of WaitForThrottleChecksComplete · a4fdbbaf
    Charlie Harrison authored
    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/961819Reviewed-by: default avataroysteine <oysteine@chromium.org>
    Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
    Commit-Queue: Charlie Harrison <csharrison@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#546184}
    a4fdbbaf
navigation_simulator_unittest.cc 6.57 KB