Commit 0d660b94 authored by Alex Clarke's avatar Alex Clarke Committed by Commit Bot

Revert "Fix MOCK_TIME bug with racy cross thread immediate task posting"

This reverts commit e7eac012.

Reason for revert: Looks like one of the tests is failing on ios

Original change's description:
> Fix MOCK_TIME bug with racy cross thread immediate task posting
> 
> In https://crrev.com/c/1564008 we have a test that only cares about main
> thread delayed tasks.  However it invokes an API which as a side effect
> triggers cross thread PostTask to the main thread.  These tasks are
> uninteresting except occasionally they happen in the window between
> MockTimeDomain::DelayTillNextTask and
> MockTimeDomain::MaybeFastForwardToNextTask.  Due to a bug in
> MaybeFastForwardToNextTask it was treating this racy cross thread post
> the same as running out of tasks and exiting early.
> 
> This is clearly undesirable and this patch fixes that behaviour.
> 
> Change-Id: I98a96c98c8ae56fd22bc2b9ea452d46565354127
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1565983
> Commit-Queue: Alex Clarke <alexclarke@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#651677}

TBR=gab@chromium.org,fdoray@chromium.org,alexclarke@chromium.org

Change-Id: Id8850f47c66400d59e8aa43e04f3bf5d25205e08
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1571687Reviewed-by: default avatarAlex Clarke <alexclarke@chromium.org>
Commit-Queue: Alex Clarke <alexclarke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#651730}
parent 258c19a7
......@@ -64,7 +64,7 @@ class BASE_EXPORT TimeDomain {
// This is the signal that virtual time should step forward. If
// RunLoop::QuitWhenIdle has been called then |quit_when_idle_requested| will
// be true. Returns true if there is a task to run now.
// be true. Returns true if time advanced and there is now a task to run.
virtual bool MaybeFastForwardToNextTask(bool quit_when_idle_requested) = 0;
protected:
......
......@@ -191,17 +191,12 @@ class ScopedTaskEnvironment::MockTimeDomain
// We don't need to call ReclaimMemory here because
// DelayTillNextTask will have dealt with cancelled delayed tasks for us.
Optional<TimeTicks> run_time = NextScheduledRunTime();
// If an immediate task came in racily from another thread, resume work
// without advancing time. This can happen regardless of whether the main
// thread has more delayed tasks scheduled before |allow_advance_until_|. If
// there are such tasks, auto-advancing time all the way would be incorrect.
// In both cases, resuming is fine.
if (run_time == now_ticks_)
return true;
if (!run_time) {
// We've run out of tasks. ScopedTaskEnvironment::FastForwardBy requires
// the remaining virtual time to be consumed upon reaching idle.
if (!run_time || run_time == now_ticks_) {
// We've run out of tasks (or an immediate task came in racily from
// another thread after reaching idle, ignore it, it will be processed in
// the next run as-if it arrived slightly later).
// ScopedTaskEnvironment::FastForwardBy requires the remaining virtual
// time to be consumed upon reaching idle.
if (now_ticks_ < allow_advance_until_ && !allow_advance_until_.is_max())
SetTime(allow_advance_until_);
return false;
......@@ -444,11 +439,6 @@ sequence_manager::TimeDomain* ScopedTaskEnvironment::GetTimeDomain() const {
: sequence_manager_->GetRealTimeDomain();
}
void ScopedTaskEnvironment::SetAllowTimeToAutoAdvanceUntilForTesting(
TimeTicks advance_until) {
mock_time_domain_->SetAllowTimeToAutoAdvanceUntil(advance_until);
}
sequence_manager::SequenceManager* ScopedTaskEnvironment::sequence_manager()
const {
DCHECK(subclass_creates_default_taskrunner_);
......
......@@ -236,9 +236,6 @@ class ScopedTaskEnvironment {
// Returns the TimeDomain driving this ScopedTaskEnvironment.
sequence_manager::TimeDomain* GetTimeDomain() const;
// For testing the MockTimeDomain.
void SetAllowTimeToAutoAdvanceUntilForTesting(TimeTicks advance_until);
sequence_manager::SequenceManager* sequence_manager() const;
void DeferredInitFromSubclass(
......
......@@ -15,12 +15,10 @@
#include "base/synchronization/waitable_event.h"
#include "base/task/post_task.h"
#include "base/task/sequence_manager/time_domain.h"
#include "base/test/bind_test_util.h"
#include "base/test/mock_callback.h"
#include "base/test/test_timeouts.h"
#include "base/threading/platform_thread.h"
#include "base/threading/sequence_local_storage_slot.h"
#include "base/threading/thread.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/clock.h"
#include "base/time/default_clock.h"
......@@ -52,7 +50,6 @@ class ScopedTaskEnvironmentForTest : public ScopedTaskEnvironment {
: ScopedTaskEnvironment(args...) {}
using ScopedTaskEnvironment::GetTimeDomain;
using ScopedTaskEnvironment::SetAllowTimeToAutoAdvanceUntilForTesting;
};
class ScopedTaskEnvironmentTest
......@@ -383,112 +380,23 @@ TEST_F(ScopedTaskEnvironmentTest, MockTimeDomain_MaybeFastForwardToNextTask) {
TEST_F(ScopedTaskEnvironmentTest,
MockTimeDomain_MaybeFastForwardToNextTask_ImmediateTaskPending) {
constexpr base::TimeDelta kDelay = TimeDelta::FromSeconds(42);
ScopedTaskEnvironmentForTest scoped_task_environment(
ScopedTaskEnvironment::MainThreadType::MOCK_TIME,
ScopedTaskEnvironment::NowSource::MAIN_THREAD_MOCK_TIME);
const TimeTicks start_time = base::TimeTicks::Now();
scoped_task_environment.SetAllowTimeToAutoAdvanceUntilForTesting(
start_time + TimeDelta::FromSeconds(100));
ThreadTaskRunnerHandle::Get()->PostDelayedTask(FROM_HERE, base::DoNothing(),
TimeDelta::FromSeconds(42));
ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, base::DoNothing());
EXPECT_TRUE(
EXPECT_FALSE(
scoped_task_environment.GetTimeDomain()->MaybeFastForwardToNextTask(
false));
EXPECT_EQ(start_time, base::TimeTicks::Now());
}
TEST_F(ScopedTaskEnvironmentTest,
MockTimeDomain_MaybeFastForwardToNextTask_TaskAfterAutoAdvanceUntil) {
constexpr base::TimeDelta kDelay = TimeDelta::FromSeconds(42);
ScopedTaskEnvironmentForTest scoped_task_environment(
ScopedTaskEnvironment::MainThreadType::MOCK_TIME,
ScopedTaskEnvironment::NowSource::MAIN_THREAD_MOCK_TIME);
const TimeTicks start_time = base::TimeTicks::Now();
scoped_task_environment.SetAllowTimeToAutoAdvanceUntilForTesting(start_time +
kDelay);
ThreadTaskRunnerHandle::Get()->PostDelayedTask(FROM_HERE, base::DoNothing(),
TimeDelta::FromSeconds(100));
EXPECT_TRUE(
scoped_task_environment.GetTimeDomain()->MaybeFastForwardToNextTask(
false));
EXPECT_EQ(start_time + kDelay, base::TimeTicks::Now());
}
TEST_F(ScopedTaskEnvironmentTest,
MockTimeDomain_MaybeFastForwardToNextTask_NoTasksPending) {
constexpr base::TimeDelta kDelay = TimeDelta::FromSeconds(42);
ScopedTaskEnvironmentForTest scoped_task_environment(
ScopedTaskEnvironment::MainThreadType::MOCK_TIME,
ScopedTaskEnvironment::NowSource::MAIN_THREAD_MOCK_TIME);
const TimeTicks start_time = base::TimeTicks::Now();
scoped_task_environment.SetAllowTimeToAutoAdvanceUntilForTesting(start_time +
kDelay);
kDelay);
ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, base::DoNothing());
EXPECT_FALSE(
scoped_task_environment.GetTimeDomain()->MaybeFastForwardToNextTask(
false));
EXPECT_EQ(start_time + kDelay, base::TimeTicks::Now());
}
TEST_F(ScopedTaskEnvironmentTest, FastForwardZero) {
ScopedTaskEnvironment scoped_task_environment(
ScopedTaskEnvironment::MainThreadType::MOCK_TIME);
int run_count = 0;
ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, BindLambdaForTesting([&]() { run_count++; }));
ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, BindLambdaForTesting([&]() { run_count++; }));
ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, BindLambdaForTesting([&]() { run_count++; }));
scoped_task_environment.FastForwardBy(base::TimeDelta());
EXPECT_EQ(3, run_count);
}
TEST_F(ScopedTaskEnvironmentTest, CrossThreadTaskPostingDoesntAffectMockTime) {
ScopedTaskEnvironment scoped_task_environment(
ScopedTaskEnvironment::MainThreadType::MOCK_TIME);
scoped_refptr<SingleThreadTaskRunner> main_thread =
ThreadTaskRunnerHandle::Get();
// Start a thread that will spam the main thread with uninteresting tasks
// which shouldn't interfere with main thread MOCK_TIME.
Thread spaming_thread("test thread");
spaming_thread.Start();
AtomicFlag stop_spamming;
RepeatingClosure repeating_spam_task = BindLambdaForTesting([&]() {
if (stop_spamming.IsSet())
return;
// We don't want to completely drown out main thread tasks so we rate limit
// how fast we post to the main thread to at most 1 per 10 microseconds.
spaming_thread.task_runner()->PostDelayedTask(
FROM_HERE, repeating_spam_task, TimeDelta::FromMicroseconds(10));
main_thread->PostTask(FROM_HERE, DoNothing());
});
spaming_thread.task_runner()->PostTask(FROM_HERE, repeating_spam_task);
// Start a repeating delayed task.
int count = 0;
RepeatingClosure repeating_delayed_task = BindLambdaForTesting([&]() {
main_thread->PostDelayedTask(FROM_HERE, repeating_delayed_task,
TimeDelta::FromSeconds(1));
count++;
});
main_thread->PostDelayedTask(FROM_HERE, repeating_delayed_task,
TimeDelta::FromSeconds(1));
scoped_task_environment.FastForwardBy(TimeDelta::FromSeconds(10000));
EXPECT_EQ(count, 10000);
stop_spamming.Set();
spaming_thread.Stop();
EXPECT_EQ(start_time, base::TimeTicks::Now());
}
#if defined(OS_WIN)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment