Skip to content

Commit

Permalink
Fix evergreen updateClient crash (youtube#3079)
Browse files Browse the repository at this point in the history
b/335726353
  • Loading branch information
sherryzy authored Apr 30, 2024
1 parent ad53292 commit 7fa46d0
Show file tree
Hide file tree
Showing 32 changed files with 286 additions and 284 deletions.
4 changes: 1 addition & 3 deletions cobalt/loader/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,7 @@ target(gtest_target_type, "loader_test") {

sources = [
"blob_fetcher_test.cc",

# TODO: b/335330660 - Re-enable FetcherCacheTests once crash is fixed for evergreen.
# "fetcher_cache_test.cc",
"fetcher_cache_test.cc",
"fetcher_factory_test.cc",
"fetcher_test.h",
"file_fetcher_test.cc",
Expand Down
50 changes: 25 additions & 25 deletions cobalt/loader/fetcher_cache_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
#include "base/memory/ref_counted.h"
#include "base/run_loop.h"
#include "base/strings/string_number_conversions.h"
#include "base/task/sequenced_task_runner.h"
#include "base/test/task_environment.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
#include "cobalt/loader/fetcher.h"
#include "net/http/http_response_headers.h"
Expand Down Expand Up @@ -150,15 +150,15 @@ TEST_F(FetcherCacheTest, SunnyDay) {
scoped_refptr<net::HttpResponseHeaders> headers =
base::MakeRefCounted<net::HttpResponseHeaders>("HTTP/1.1 200 OK\n");

base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireResponseStarted,
base::Unretained(stub_fetcher_factory.fetcher), headers));
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE, base::BindOnce(&StubFetcher::FireReceived,
base::Unretained(stub_fetcher_factory.fetcher),
data_.c_str(), data_.size()));
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireDone,
base::Unretained(stub_fetcher_factory.fetcher)));
Expand Down Expand Up @@ -187,7 +187,7 @@ TEST_F(FetcherCacheTest, DuplicateFetcherCache) {
stub_fetcher_factorys.push_back(stub_fetcher_factory);
fetcher_creators.push_back(fetcher_creator);
fetchers.push_back(std::move(fetcher));
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireFetcherProcess,
base::Unretained(stub_fetcher_factorys[i].fetcher),
Expand Down Expand Up @@ -219,7 +219,7 @@ TEST_F(FetcherCacheTest, MultipleFetcherCache) {
stub_fetcher_factorys.push_back(stub_fetcher_factory);
fetcher_creators.push_back(fetcher_creator);
fetchers.push_back(std::move(fetcher));
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireFetcherProcess,
base::Unretained(stub_fetcher_factorys[i].fetcher),
Expand Down Expand Up @@ -255,7 +255,7 @@ TEST_F(FetcherCacheTest, MaxFetcherCache) {
stub_fetcher_factorys.push_back(stub_fetcher_factory);
fetcher_creators.push_back(fetcher_creator);
fetchers.push_back(std::move(fetcher));
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireFetcherProcess,
base::Unretained(stub_fetcher_factorys[i].fetcher),
Expand Down Expand Up @@ -284,30 +284,30 @@ TEST_F(FetcherCacheTest, DisorderedSameFetcherCache) {
std::unique_ptr<Fetcher> fetcher2 = fetcher_creator2.Run(&handler);

// Task 1: start and receive
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireResponseStarted,
base::Unretained(stub_fetcher_factory1.fetcher), headers));
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE, base::BindOnce(&StubFetcher::FireReceived,
base::Unretained(stub_fetcher_factory1.fetcher),
data_.c_str(), data_.size()));
// Task 2: start and receive
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireResponseStarted,
base::Unretained(stub_fetcher_factory2.fetcher), headers));
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE, base::BindOnce(&StubFetcher::FireReceived,
base::Unretained(stub_fetcher_factory2.fetcher),
data_.c_str(), data_.size()));
// Task 1: done
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireDone,
base::Unretained(stub_fetcher_factory1.fetcher)));
// Task 2: done
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireDone,
base::Unretained(stub_fetcher_factory2.fetcher)));
Expand All @@ -334,30 +334,30 @@ TEST_F(FetcherCacheTest, DisorderedDiffFetcherCache) {
std::unique_ptr<Fetcher> fetcher2 = fetcher_creator2.Run(&handler);

// Task 1: start and receive
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireResponseStarted,
base::Unretained(stub_fetcher_factory1.fetcher), headers));
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE, base::BindOnce(&StubFetcher::FireReceived,
base::Unretained(stub_fetcher_factory1.fetcher),
data_.c_str(), data_.size()));
// Task 2: start and receive
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireResponseStarted,
base::Unretained(stub_fetcher_factory2.fetcher), headers));
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE, base::BindOnce(&StubFetcher::FireReceived,
base::Unretained(stub_fetcher_factory2.fetcher),
data_.c_str(), data_.size()));
// Task 1: done
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireDone,
base::Unretained(stub_fetcher_factory1.fetcher)));
// Task 2: done
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireDone,
base::Unretained(stub_fetcher_factory2.fetcher)));
Expand All @@ -384,29 +384,29 @@ TEST_F(FetcherCacheTest, DisorderedSlowFetcherCache) {
std::unique_ptr<Fetcher> fetcher2 = fetcher_creator2.Run(&handler);

// Task 1: start and receive
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireResponseStarted,
base::Unretained(stub_fetcher_factory1.fetcher), headers));
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE, base::BindOnce(&StubFetcher::FireReceived,
base::Unretained(stub_fetcher_factory1.fetcher),
data_.c_str(), data_.size()));
// Task 2: start, receive, and done
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireResponseStarted,
base::Unretained(stub_fetcher_factory2.fetcher), headers));
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE, base::BindOnce(&StubFetcher::FireReceived,
base::Unretained(stub_fetcher_factory2.fetcher),
data_.c_str(), data_.size()));
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireDone,
base::Unretained(stub_fetcher_factory2.fetcher)));
// Task 1: done
base::ThreadTaskRunnerHandle::Get()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&StubFetcher::FireDone,
base::Unretained(stub_fetcher_factory1.fetcher)));
Expand Down
1 change: 0 additions & 1 deletion components/update_client/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ static_library("test_support") {

if (cobalt_pending_clean_up) {
sources -= [
"net/network_impl_cobalt.cc",
"net/url_request_post_interceptor.cc",
]
}
Expand Down
6 changes: 3 additions & 3 deletions components/update_client/action_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ namespace update_client {
ActionRunner::ActionRunner(const Component& component)
: is_per_user_install_(component.config()->IsPerUserInstall()),
component_(component),
main_task_runner_(base::ThreadTaskRunnerHandle::Get()) {}
main_task_runner_(base::SequencedTaskRunner::GetCurrentDefault()) {}

ActionRunner::~ActionRunner() {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
}

void ActionRunner::Run(Callback run_complete) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

run_complete_ = std::move(run_complete);

Expand Down
6 changes: 3 additions & 3 deletions components/update_client/action_runner.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#include "base/files/file_path.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/threading/thread_checker.h"
#include "base/sequence_checker.h"
#include "build/build_config.h"
#include "components/update_client/component_unpacker.h"

Expand Down Expand Up @@ -59,14 +59,14 @@ class ActionRunner {
const Component& component_;

// Used to post callbacks to the main thread.
scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
scoped_refptr<base::SequencedTaskRunner> main_task_runner_;

// Contains the unpack path for the component associated with the run action.
base::FilePath unpack_path_;

Callback run_complete_;

THREAD_CHECKER(thread_checker_);
SEQUENCE_CHECKER(sequence_checker_);
DISALLOW_COPY_AND_ASSIGN(ActionRunner);
};

Expand Down
Loading

0 comments on commit 7fa46d0

Please sign in to comment.