Update UE to remove MessageLoop::current()->WatchFileDescriptor.
MessageLoop::current()->WatchFileDescriptor is deprecated. And UE should
remove usages of it.
Test: mma && unittest
Change-Id: Ib1ef2e6b6a38ad2a8d07b78bcd72fdb3b7f82226
diff --git a/common/subprocess.cc b/common/subprocess.cc
index 298a65c..3e197fb 100644
--- a/common/subprocess.cc
+++ b/common/subprocess.cc
@@ -129,12 +129,7 @@
if (!ok || eof) {
// There was either an error or an EOF condition, so we are done watching
// the file descriptor.
-#ifdef __ANDROID__
- MessageLoop::current()->CancelTask(record->stdout_task_id);
- record->stdout_task_id = MessageLoop::kTaskIdNull;
-#else
record->stdout_controller.reset();
-#endif // __ANDROID__
return;
}
} while (bytes_read);
@@ -149,12 +144,7 @@
// Make sure we read any remaining process output and then close the pipe.
OnStdoutReady(record);
-#ifdef __ANDROID__
- MessageLoop::current()->CancelTask(record->stdout_task_id);
- record->stdout_task_id = MessageLoop::kTaskIdNull;
-#else
record->stdout_controller.reset();
-#endif // __ANDROID__
// Don't print any log if the subprocess exited with exit code 0.
if (info.si_code != CLD_EXITED) {
@@ -209,18 +199,9 @@
<< record->stdout_fd << ".";
}
-#ifdef __ANDROID__
- record->stdout_task_id = MessageLoop::current()->WatchFileDescriptor(
- FROM_HERE,
- record->stdout_fd,
- MessageLoop::WatchMode::kWatchRead,
- true,
- base::Bind(&Subprocess::OnStdoutReady, record.get()));
-#else
record->stdout_controller = base::FileDescriptorWatcher::WatchReadable(
record->stdout_fd,
base::BindRepeating(&Subprocess::OnStdoutReady, record.get()));
-#endif // __ANDROID__
subprocess_records_[pid] = std::move(record);
return pid;
diff --git a/common/subprocess.h b/common/subprocess.h
index f1b9f1f..432d4cb 100644
--- a/common/subprocess.h
+++ b/common/subprocess.h
@@ -123,12 +123,8 @@
// These are used to monitor the stdout of the running process, including
// the stderr if it was redirected.
-#ifdef __ANDROID__
- brillo::MessageLoop::TaskId stdout_task_id{
- brillo::MessageLoop::kTaskIdNull};
-#else
std::unique_ptr<base::FileDescriptorWatcher::Controller> stdout_controller;
-#endif // __ANDROID__
+
int stdout_fd{-1};
std::string stdout;
};
diff --git a/common/subprocess_unittest.cc b/common/subprocess_unittest.cc
index bc52b83..e71d3d8 100644
--- a/common/subprocess_unittest.cc
+++ b/common/subprocess_unittest.cc
@@ -74,9 +74,7 @@
brillo::BaseMessageLoop loop_{&base_loop_};
brillo::AsynchronousSignalHandler async_signal_handler_;
Subprocess subprocess_;
-#ifndef __ANDROID__
unique_ptr<base::FileDescriptorWatcher::Controller> watcher_;
-#endif // __ANDROID__
};
@@ -261,23 +259,6 @@
int fifo_fd = HANDLE_EINTR(open(fifo_path.c_str(), O_RDONLY));
EXPECT_GE(fifo_fd, 0);
-#ifdef __ANDROID__
- loop_.WatchFileDescriptor(FROM_HERE,
- fifo_fd,
- MessageLoop::WatchMode::kWatchRead,
- false,
- base::Bind(
- [](int fifo_fd, uint32_t tag) {
- char c;
- EXPECT_EQ(1,
- HANDLE_EINTR(read(fifo_fd, &c, 1)));
- EXPECT_EQ('X', c);
- LOG(INFO) << "Killing tag " << tag;
- Subprocess::Get().KillExec(tag);
- },
- fifo_fd,
- tag));
-#else
watcher_ = base::FileDescriptorWatcher::WatchReadable(
fifo_fd,
base::Bind(
@@ -295,7 +276,6 @@
base::Unretained(&watcher_),
fifo_fd,
tag));
-#endif // __ANDROID__
// This test would leak a callback that runs when the child process exits
// unless we wait for it to run.
diff --git a/libcurl_http_fetcher.cc b/libcurl_http_fetcher.cc
index 7c53a2d..f8aed7c 100644
--- a/libcurl_http_fetcher.cc
+++ b/libcurl_http_fetcher.cc
@@ -31,6 +31,8 @@
#include <base/strings/string_split.h>
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
+#include <base/threading/thread_task_runner_handle.h>
+
#ifdef __ANDROID__
#include <cutils/qtaguid.h>
@@ -81,23 +83,9 @@
LibcurlHttpFetcher* fetcher = static_cast<LibcurlHttpFetcher*>(clientp);
// Stop watching the socket before closing it.
-#ifdef __ANDROID__
- for (size_t t = 0; t < arraysize(fetcher->fd_task_maps_); ++t) {
- const auto fd_task_pair = fetcher->fd_task_maps_[t].find(item);
- if (fd_task_pair != fetcher->fd_task_maps_[t].end()) {
- if (!MessageLoop::current()->CancelTask(fd_task_pair->second)) {
- LOG(WARNING) << "Error canceling the watch task "
- << fd_task_pair->second << " for "
- << (t ? "writing" : "reading") << " the fd " << item;
- }
- fetcher->fd_task_maps_[t].erase(item);
- }
- }
-#else
for (size_t t = 0; t < base::size(fetcher->fd_controller_maps_); ++t) {
fetcher->fd_controller_maps_[t].erase(item);
}
-#endif // __ANDROID__
// Documentation for this callback says to return 0 on success or 1 on error.
if (!IGNORE_EINTR(close(item)))
@@ -471,6 +459,19 @@
// There's either more work to do or we are paused, so we just keep the
// file descriptors to watch up to date and exit, until we are done with the
// work and we are not paused.
+#ifdef __ANDROID__
+ // When there's no base::SingleThreadTaskRunner on current thread, it's not
+ // possible to watch file descriptors. Just poll it later. This usually
+ // happens if brillo::FakeMessageLoop is used.
+ if (!base::ThreadTaskRunnerHandle::IsSet()) {
+ MessageLoop::current()->PostDelayedTask(
+ FROM_HERE,
+ base::Bind(&LibcurlHttpFetcher::CurlPerformOnce,
+ base::Unretained(this)),
+ TimeDelta::FromSeconds(1));
+ return;
+ }
+#endif
SetupMessageLoopSources();
return;
}
@@ -691,63 +692,6 @@
// We should iterate through all file descriptors up to libcurl's fd_max or
// the highest one we're tracking, whichever is larger.
-#ifdef __ANDROID__
- for (size_t t = 0; t < arraysize(fd_task_maps_); ++t) {
- if (!fd_task_maps_[t].empty())
- fd_max = max(fd_max, fd_task_maps_[t].rbegin()->first);
- }
-
- // For each fd, if we're not tracking it, track it. If we are tracking it, but
- // libcurl doesn't care about it anymore, stop tracking it. After this loop,
- // there should be exactly as many tasks scheduled in fd_task_maps_[0|1] as
- // there are read/write fds that we're tracking.
- for (int fd = 0; fd <= fd_max; ++fd) {
- // Note that fd_exc is unused in the current version of libcurl so is_exc
- // should always be false.
- bool is_exc = FD_ISSET(fd, &fd_exc) != 0;
- bool must_track[2] = {
- is_exc || (FD_ISSET(fd, &fd_read) != 0), // track 0 -- read
- is_exc || (FD_ISSET(fd, &fd_write) != 0) // track 1 -- write
- };
- MessageLoop::WatchMode watch_modes[2] = {
- MessageLoop::WatchMode::kWatchRead,
- MessageLoop::WatchMode::kWatchWrite,
- };
-
- for (size_t t = 0; t < arraysize(fd_task_maps_); ++t) {
- auto fd_task_it = fd_task_maps_[t].find(fd);
- bool tracked = fd_task_it != fd_task_maps_[t].end();
-
- if (!must_track[t]) {
- // If we have an outstanding io_channel, remove it.
- if (tracked) {
- MessageLoop::current()->CancelTask(fd_task_it->second);
- fd_task_maps_[t].erase(fd_task_it);
- }
- continue;
- }
-
- // If we are already tracking this fd, continue -- nothing to do.
- if (tracked)
- continue;
-
- // Track a new fd.
- fd_task_maps_[t][fd] = MessageLoop::current()->WatchFileDescriptor(
- FROM_HERE,
- fd,
- watch_modes[t],
- true, // persistent
- base::Bind(&LibcurlHttpFetcher::CurlPerformOnce,
- base::Unretained(this)));
-
- static int io_counter = 0;
- io_counter++;
- if (io_counter % 50 == 0) {
- LOG(INFO) << "io_counter = " << io_counter;
- }
- }
- }
-#else
for (size_t t = 0; t < base::size(fd_controller_maps_); ++t) {
if (!fd_controller_maps_[t].empty())
fd_max = max(fd_max, fd_controller_maps_[t].rbegin()->first);
@@ -803,7 +747,6 @@
}
}
}
-#endif // __ANDROID__
// Set up a timeout callback for libcurl.
if (timeout_id_ == MessageLoop::kTaskIdNull) {
@@ -848,22 +791,9 @@
MessageLoop::current()->CancelTask(timeout_id_);
timeout_id_ = MessageLoop::kTaskIdNull;
-#ifdef __ANDROID__
- for (size_t t = 0; t < arraysize(fd_task_maps_); ++t) {
- for (const auto& fd_taks_pair : fd_task_maps_[t]) {
- if (!MessageLoop::current()->CancelTask(fd_taks_pair.second)) {
- LOG(WARNING) << "Error canceling the watch task " << fd_taks_pair.second
- << " for " << (t ? "writing" : "reading") << " the fd "
- << fd_taks_pair.first;
- }
- }
- fd_task_maps_[t].clear();
- }
-#else
for (size_t t = 0; t < base::size(fd_controller_maps_); ++t) {
fd_controller_maps_[t].clear();
}
-#endif // __ANDROID__
if (curl_http_headers_) {
curl_slist_free_all(curl_http_headers_);
diff --git a/libcurl_http_fetcher.h b/libcurl_http_fetcher.h
index 4854f40..4e91b69 100644
--- a/libcurl_http_fetcher.h
+++ b/libcurl_http_fetcher.h
@@ -255,12 +255,8 @@
// the message loop. libcurl may open/close descriptors and switch their
// directions so maintain two separate lists so that watch conditions can be
// set appropriately.
-#ifdef __ANDROID__
- std::map<int, brillo::MessageLoop::TaskId> fd_task_maps_[2];
-#else
std::map<int, std::unique_ptr<base::FileDescriptorWatcher::Controller>>
fd_controller_maps_[2];
-#endif // __ANDROID__
// The TaskId of the timer we're waiting on. kTaskIdNull if we are not waiting
// on it.
diff --git a/libcurl_http_fetcher_unittest.cc b/libcurl_http_fetcher_unittest.cc
index 8064b99..874ef2e 100644
--- a/libcurl_http_fetcher_unittest.cc
+++ b/libcurl_http_fetcher_unittest.cc
@@ -94,37 +94,24 @@
no_network_max_retries);
}
-#ifdef __ANDROID__
-TEST_F(LibcurlHttpFetcherTest, CouldntResolveHostTest) {
- int no_network_max_retries = 1;
- libcurl_fetcher_.set_no_network_max_retries(no_network_max_retries);
-
- // This test actually sends request to internet but according to
- // https://tools.ietf.org/html/rfc2606#section-2, .invalid domain names are
- // reserved and sure to be invalid. Ideally we should mock libcurl or
- // reorganize LibcurlHttpFetcher so the part that sends request can be mocked
- // easily.
- // TODO(xiaochu) Refactor LibcurlHttpFetcher (and its relates) so it's
- // easier to mock the part that depends on internet connectivity.
- libcurl_fetcher_.BeginTransfer("https://An-uNres0lvable-uRl.invalid");
- while (loop_.PendingTasks()) {
- loop_.RunOnce(true);
- }
-
- // If libcurl fails to resolve the name, we call res_init() to reload
- // resolv.conf and retry exactly once more. See crbug.com/982813 for details.
- EXPECT_EQ(libcurl_fetcher_.get_no_network_max_retries(),
- no_network_max_retries + 1);
-}
-#else
TEST_F(LibcurlHttpFetcherTest, CouldNotResolveHostTest) {
int no_network_max_retries = 1;
libcurl_fetcher_.set_no_network_max_retries(no_network_max_retries);
libcurl_fetcher_.BeginTransfer("https://An-uNres0lvable-uRl.invalid");
+#ifdef __ANDROID__
+ // It's slower on Android that libcurl handle may not finish within 1 cycle.
+ // Will need to wait for more cycles until it finishes. Original test didn't
+ // correctly handle when we need to re-watch libcurl fds.
+ while (loop_.PendingTasks() &&
+ libcurl_fetcher_.GetAuxiliaryErrorCode() == ErrorCode::kSuccess) {
+ loop_.RunOnce(true);
+ }
+#else
// The first time it can't resolve.
loop_.RunOnce(true);
+#endif
EXPECT_EQ(libcurl_fetcher_.GetAuxiliaryErrorCode(),
ErrorCode::kUnresolvedHostError);
@@ -154,8 +141,18 @@
// easier to mock the part that depends on internet connectivity.
libcurl_fetcher_.BeginTransfer("https://An-uNres0lvable-uRl.invalid");
+#ifdef __ANDROID__
+ // It's slower on Android that libcurl handle may not finish within 1 cycle.
+ // Will need to wait for more cycles until it finishes. Original test didn't
+ // correctly handle when we need to re-watch libcurl fds.
+ while (loop_.PendingTasks() &&
+ libcurl_fetcher_.GetAuxiliaryErrorCode() == ErrorCode::kSuccess) {
+ loop_.RunOnce(true);
+ }
+#else
// The first time it can't resolve.
loop_.RunOnce(true);
+#endif
EXPECT_EQ(libcurl_fetcher_.GetAuxiliaryErrorCode(),
ErrorCode::kUnresolvedHostError);
@@ -168,9 +165,19 @@
[this]() { libcurl_fetcher_.http_response_code_ = 0; }));
libcurl_fetcher_.transfer_size_ = 10;
+#ifdef __ANDROID__
+ // It's slower on Android that libcurl handle may not finish within 1 cycle.
+ // Will need to wait for more cycles until it finishes. Original test didn't
+ // correctly handle when we need to re-watch libcurl fds.
+ while (loop_.PendingTasks() && libcurl_fetcher_.GetAuxiliaryErrorCode() ==
+ ErrorCode::kUnresolvedHostError) {
+ loop_.RunOnce(true);
+ }
+#else
// This time the host is resolved. But after that again we can't resolve
// anymore (See above).
loop_.RunOnce(true);
+#endif
EXPECT_EQ(libcurl_fetcher_.GetAuxiliaryErrorCode(),
ErrorCode::kUnresolvedHostRecovered);
@@ -186,7 +193,6 @@
EXPECT_EQ(libcurl_fetcher_.get_no_network_max_retries(),
no_network_max_retries + 1);
}
-#endif // __ANDROID__
TEST_F(LibcurlHttpFetcherTest, HttpFetcherStateMachineRetryFailedTest) {
state_machine_.UpdateState(true);
diff --git a/payload_consumer/postinstall_runner_action.cc b/payload_consumer/postinstall_runner_action.cc
index c520c7e..94d0392 100644
--- a/payload_consumer/postinstall_runner_action.cc
+++ b/payload_consumer/postinstall_runner_action.cc
@@ -220,20 +220,10 @@
PLOG(ERROR) << "Unable to set non-blocking I/O mode on fd " << progress_fd_;
}
-#ifdef __ANDROID__
- progress_task_ = MessageLoop::current()->WatchFileDescriptor(
- FROM_HERE,
- progress_fd_,
- MessageLoop::WatchMode::kWatchRead,
- true,
- base::Bind(&PostinstallRunnerAction::OnProgressFdReady,
- base::Unretained(this)));
-#else
progress_controller_ = base::FileDescriptorWatcher::WatchReadable(
progress_fd_,
base::BindRepeating(&PostinstallRunnerAction::OnProgressFdReady,
base::Unretained(this)));
-#endif // __ANDROID__
}
@@ -259,12 +249,7 @@
if (!ok || eof) {
// There was either an error or an EOF condition, so we are done watching
// the file descriptor.
-#ifdef __ANDROID__
- MessageLoop::current()->CancelTask(progress_task_);
- progress_task_ = MessageLoop::kTaskIdNull;
-#else
progress_controller_.reset();
-#endif // __ANDROID__
return;
}
} while (bytes_read);
@@ -308,14 +293,7 @@
fs_mount_dir_.clear();
progress_fd_ = -1;
-#ifdef __ANDROID__
- if (progress_task_ != MessageLoop::kTaskIdNull) {
- MessageLoop::current()->CancelTask(progress_task_);
- progress_task_ = MessageLoop::kTaskIdNull;
- }
-#else
progress_controller_.reset();
-#endif // __ANDROID__
progress_buffer_.clear();
}
diff --git a/payload_consumer/postinstall_runner_action.h b/payload_consumer/postinstall_runner_action.h
index bbc9e8c..e404107 100644
--- a/payload_consumer/postinstall_runner_action.h
+++ b/payload_consumer/postinstall_runner_action.h
@@ -140,11 +140,7 @@
// the postinstall program and the task watching for them.
int progress_fd_{-1};
-#ifdef __ANDROID__
- brillo::MessageLoop::TaskId progress_task_{brillo::MessageLoop::kTaskIdNull};
-#else
std::unique_ptr<base::FileDescriptorWatcher::Controller> progress_controller_;
-#endif // __ANDROID__
// A buffer of a partial read line from the progress file descriptor.
std::string progress_buffer_;
