Skip to content
  • gab's avatar
    Introduce ThreadTaskRunnerHandle::OverrideForTesting and TestMockTimeTaskRunner::ScopedContext. · a6f72328
    gab authored
    Will be used to provide proper ThreadTaskRunnerHandle context in unit
    tests when multiple test task runners share the main thread.
    
    TestMockTimeTaskRunner::ScopedContext as discussed @
    https://groups.google.com/a/chromium.org/d/msg/chromium-dev/kSIe9p5ZYGM/koFefJmNCAAJ
    
    This cleans up HttpServerPropertiesManagerTest to actually run code
    in the scope of the task runners the impl expects it to (it would
    previously pass because TestMockTimeTaskRunner::RunsOnCurrentThread()
    is looser than it should be -- merely checks thread id, not runner context).
    This is a prerequisite for https://codereview.chromium.org/2491613004/
    as it otherwise breaks per its tasks posting to unexpected tasks runners
    (i.e. the current ThreadTaskRunnerHandle() which was the underlying unmocked
    MessageLoop in these tests).
    
    Further addressed in those tests:
     - No longer need to use MessageLoop/RunLoop
       - Which removes need for completion Closure on the updates.
     - Exposes timer timings to fast-forward by the right timing instead of
       FastForwardUntilNoTasksRemain() -- test could use some more cleanup in that
       regard but that's a first pass of mandatory sites.
    
    Also addressed in this CL:
     - ServerBackedStateKeysBrokerTest.Refresh, RecentTabHelperTest, AffiliatedMatchHelperTest, and SyncStoppedReporterTest
      - Now mocks main thread runner directly, removes need for test-only SetTaskRunner() methods :).
       (having a custom TaskRunner was causing things like observers to post to the wrong task runner per this CL's
        ThreadTaskRunnerHandle override in its deferred initialization).
     - AsyncDocumentSubresourceFilterTest, WallPaperColorCalculatorTest and PostAndReplyImplTest
      - Unfortunately those tests were fine but as documented in thread_task_runner_handle.cc:
        supporting SequencedTaskRunnerHandle overrides from main thread would be overkill for
        now... so they were tweaked to avoid doing that.
      - Also, upcoming base::test::ScopedTaskEnvironment makes all of this an implementation detail anyways:
        draft @ https://docs.google.com/document/d/1QabRo8c7D9LsYY3cEcaPQbOCLo8Tu-6VLykYXyl3Pkk/edit
    
    BUG=587199
    
    Review-Url: https://codereview.chromium.org/2657013002
    Cr-Commit-Position: refs/heads/master@{#454127}
    a6f72328