diff options
author | Daniel Erat <derat@chromium.org> | 2018-04-24 13:49:35 -0700 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2018-05-02 22:21:13 -0700 |
commit | 514938dc6cc0ebcbd397764b2a59c9eec796cb97 (patch) | |
tree | a15d46c0743eb1c322d0a014de6244800ae3e432 | |
parent | a1880bb77cc066656d60af99bf1ba44eb522f08a (diff) | |
download | platform_external_libbrillo-514938dc6cc0ebcbd397764b2a59c9eec796cb97.tar.gz platform_external_libbrillo-514938dc6cc0ebcbd397764b2a59c9eec796cb97.tar.bz2 platform_external_libbrillo-514938dc6cc0ebcbd397764b2a59c9eec796cb97.zip |
libbrillo: Delete GlibMessageLoop.
This class is no longer used.
BUG=chromium:361635
TEST=built with FEATURES=test, both with and without dbus
USE flag
Change-Id: I6f96848ba1d137c62396ab2a32b35152ef79040c
Reviewed-on: https://chromium-review.googlesource.com/1026843
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Alex Deymo <deymo@google.com>
-rw-r--r-- | brillo/message_loops/glib_message_loop.cc | 194 | ||||
-rw-r--r-- | brillo/message_loops/glib_message_loop.h | 83 | ||||
-rw-r--r-- | brillo/message_loops/glib_message_loop_unittest.cc | 69 | ||||
-rw-r--r-- | brillo/message_loops/message_loop_unittest.cc | 10 | ||||
-rw-r--r-- | libbrillo.gypi | 8 |
5 files changed, 3 insertions, 361 deletions
diff --git a/brillo/message_loops/glib_message_loop.cc b/brillo/message_loops/glib_message_loop.cc deleted file mode 100644 index 20c271d..0000000 --- a/brillo/message_loops/glib_message_loop.cc +++ /dev/null @@ -1,194 +0,0 @@ -// Copyright 2015 The Chromium OS Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include <brillo/message_loops/glib_message_loop.h> - -#include <fcntl.h> -#include <unistd.h> - -#include <brillo/location_logging.h> - -using base::Closure; - -namespace brillo { - -GlibMessageLoop::GlibMessageLoop() { - loop_ = g_main_loop_new(g_main_context_default(), FALSE); -} - -GlibMessageLoop::~GlibMessageLoop() { - // Cancel all pending tasks when destroying the message loop. - for (const auto& task : tasks_) { - DVLOG_LOC(task.second->location, 1) - << "Removing task_id " << task.second->task_id - << " leaked on GlibMessageLoop, scheduled from this location."; - g_source_remove(task.second->source_id); - } - g_main_loop_unref(loop_); -} - -MessageLoop::TaskId GlibMessageLoop::PostDelayedTask( - const tracked_objects::Location& from_here, - const Closure &task, - base::TimeDelta delay) { - TaskId task_id = NextTaskId(); - // Note: While we store persistent = false in the ScheduledTask object, we - // don't check it in OnRanPostedTask() since it is always false for delayed - // tasks. This is only used for WatchFileDescriptor below. - ScheduledTask* scheduled_task = new ScheduledTask{ - this, from_here, task_id, 0, false, std::move(task)}; - DVLOG_LOC(from_here, 1) << "Scheduling delayed task_id " << task_id - << " to run in " << delay << "."; - scheduled_task->source_id = g_timeout_add_full( - G_PRIORITY_DEFAULT, - delay.InMillisecondsRoundedUp(), - &GlibMessageLoop::OnRanPostedTask, - reinterpret_cast<gpointer>(scheduled_task), - DestroyPostedTask); - tasks_[task_id] = scheduled_task; - return task_id; -} - -MessageLoop::TaskId GlibMessageLoop::WatchFileDescriptor( - const tracked_objects::Location& from_here, - int fd, - WatchMode mode, - bool persistent, - const Closure &task) { - // Quick check to see if the fd is valid. - if (fcntl(fd, F_GETFD) == -1 && errno == EBADF) - return MessageLoop::kTaskIdNull; - - GIOCondition condition = G_IO_NVAL; - switch (mode) { - case MessageLoop::kWatchRead: - condition = static_cast<GIOCondition>(G_IO_IN | G_IO_HUP | G_IO_NVAL); - break; - case MessageLoop::kWatchWrite: - condition = static_cast<GIOCondition>(G_IO_OUT | G_IO_HUP | G_IO_NVAL); - break; - default: - return MessageLoop::kTaskIdNull; - } - - // TODO(deymo): Used g_unix_fd_add_full() instead of g_io_add_watch_full() - // when/if we switch to glib 2.36 or newer so we don't need to create a - // GIOChannel for this. - GIOChannel* io_channel = g_io_channel_unix_new(fd); - if (!io_channel) - return MessageLoop::kTaskIdNull; - GError* error = nullptr; - GIOStatus status = g_io_channel_set_encoding(io_channel, nullptr, &error); - if (status != G_IO_STATUS_NORMAL) { - LOG(ERROR) << "GError(" << error->code << "): " - << (error->message ? error->message : "(unknown)"); - g_error_free(error); - // g_io_channel_set_encoding() documentation states that this should be - // valid in this context (a new io_channel), but enforce the check in - // debug mode. - DCHECK(status == G_IO_STATUS_NORMAL); - return MessageLoop::kTaskIdNull; - } - - TaskId task_id = NextTaskId(); - ScheduledTask* scheduled_task = new ScheduledTask{ - this, from_here, task_id, 0, persistent, std::move(task)}; - scheduled_task->source_id = g_io_add_watch_full( - io_channel, - G_PRIORITY_DEFAULT, - condition, - &GlibMessageLoop::OnWatchedFdReady, - reinterpret_cast<gpointer>(scheduled_task), - DestroyPostedTask); - // g_io_add_watch_full() increases the reference count on the newly created - // io_channel, so we can dereference it now and it will be free'd once the - // source is removed or now if g_io_add_watch_full() failed. - g_io_channel_unref(io_channel); - - DVLOG_LOC(from_here, 1) - << "Watching fd " << fd << " for " - << (mode == MessageLoop::kWatchRead ? "reading" : "writing") - << (persistent ? " persistently" : " just once") - << " as task_id " << task_id - << (scheduled_task->source_id ? " successfully" : " failed."); - - if (!scheduled_task->source_id) { - delete scheduled_task; - return MessageLoop::kTaskIdNull; - } - tasks_[task_id] = scheduled_task; - return task_id; -} - -bool GlibMessageLoop::CancelTask(TaskId task_id) { - if (task_id == kTaskIdNull) - return false; - const auto task = tasks_.find(task_id); - // It is a programmer error to attempt to remove a non-existent source. - if (task == tasks_.end()) - return false; - DVLOG_LOC(task->second->location, 1) - << "Removing task_id " << task_id << " scheduled from this location."; - guint source_id = task->second->source_id; - // We remove here the entry from the tasks_ map, the pointer will be deleted - // by the g_source_remove() call. - tasks_.erase(task); - return g_source_remove(source_id); -} - -bool GlibMessageLoop::RunOnce(bool may_block) { - return g_main_context_iteration(nullptr, may_block); -} - -void GlibMessageLoop::Run() { - g_main_loop_run(loop_); -} - -void GlibMessageLoop::BreakLoop() { - g_main_loop_quit(loop_); -} - -MessageLoop::TaskId GlibMessageLoop::NextTaskId() { - TaskId res; - do { - res = ++last_id_; - // We would run out of memory before we run out of task ids. - } while (!res || tasks_.find(res) != tasks_.end()); - return res; -} - -gboolean GlibMessageLoop::OnRanPostedTask(gpointer user_data) { - ScheduledTask* scheduled_task = reinterpret_cast<ScheduledTask*>(user_data); - DVLOG_LOC(scheduled_task->location, 1) - << "Running delayed task_id " << scheduled_task->task_id - << " scheduled from this location."; - // We only need to remove this task_id from the map. DestroyPostedTask will be - // called with this same |user_data| where we can delete the ScheduledTask. - scheduled_task->loop->tasks_.erase(scheduled_task->task_id); - scheduled_task->closure.Run(); - return FALSE; // Removes the source since a callback can only be called once. -} - -gboolean GlibMessageLoop::OnWatchedFdReady(GIOChannel *source, - GIOCondition condition, - gpointer user_data) { - ScheduledTask* scheduled_task = reinterpret_cast<ScheduledTask*>(user_data); - DVLOG_LOC(scheduled_task->location, 1) - << "Running task_id " << scheduled_task->task_id - << " for watching a file descriptor, scheduled from this location."; - if (!scheduled_task->persistent) { - // We only need to remove this task_id from the map. DestroyPostedTask will - // be called with this same |user_data| where we can delete the - // ScheduledTask. - scheduled_task->loop->tasks_.erase(scheduled_task->task_id); - } - scheduled_task->closure.Run(); - return scheduled_task->persistent; -} - -void GlibMessageLoop::DestroyPostedTask(gpointer user_data) { - delete reinterpret_cast<ScheduledTask*>(user_data); -} - -} // namespace brillo diff --git a/brillo/message_loops/glib_message_loop.h b/brillo/message_loops/glib_message_loop.h deleted file mode 100644 index 50fe2ce..0000000 --- a/brillo/message_loops/glib_message_loop.h +++ /dev/null @@ -1,83 +0,0 @@ -// Copyright 2015 The Chromium OS Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef LIBBRILLO_BRILLO_MESSAGE_LOOPS_GLIB_MESSAGE_LOOP_H_ -#define LIBBRILLO_BRILLO_MESSAGE_LOOPS_GLIB_MESSAGE_LOOP_H_ - -#include <map> -#include <memory> - -#include <base/location.h> -#include <base/time/time.h> -#include <glib.h> - -#include <brillo/brillo_export.h> -#include <brillo/message_loops/message_loop.h> - -namespace brillo { - -class BRILLO_EXPORT GlibMessageLoop : public MessageLoop { - public: - GlibMessageLoop(); - ~GlibMessageLoop() override; - - // MessageLoop overrides. - TaskId PostDelayedTask(const tracked_objects::Location& from_here, - const base::Closure& task, - base::TimeDelta delay) override; - using MessageLoop::PostDelayedTask; - TaskId WatchFileDescriptor(const tracked_objects::Location& from_here, - int fd, - WatchMode mode, - bool persistent, - const base::Closure& task) override; - using MessageLoop::WatchFileDescriptor; - bool CancelTask(TaskId task_id) override; - bool RunOnce(bool may_block) override; - void Run() override; - void BreakLoop() override; - - private: - // Called by the GLib's main loop when is time to call the callback scheduled - // with Post*Task(). The pointer to the callback passed when scheduling it is - // passed to this function as a gpointer on |user_data|. - static gboolean OnRanPostedTask(gpointer user_data); - - // Called by the GLib's main loop when the watched source |source| is - // ready to perform the operation given in |condition| without blocking. - static gboolean OnWatchedFdReady(GIOChannel *source, - GIOCondition condition, - gpointer user_data); - - // Called by the GLib's main loop when the scheduled callback is removed due - // to it being executed or canceled. - static void DestroyPostedTask(gpointer user_data); - - // Return a new unused task_id. - TaskId NextTaskId(); - - GMainLoop* loop_; - - struct ScheduledTask { - // A pointer to this GlibMessageLoop so we can remove the Task from the - // glib callback. - GlibMessageLoop* loop; - tracked_objects::Location location; - - MessageLoop::TaskId task_id; - guint source_id; - bool persistent; - base::Closure closure; - }; - - std::map<MessageLoop::TaskId, ScheduledTask*> tasks_; - - MessageLoop::TaskId last_id_ = kTaskIdNull; - - DISALLOW_COPY_AND_ASSIGN(GlibMessageLoop); -}; - -} // namespace brillo - -#endif // LIBBRILLO_BRILLO_MESSAGE_LOOPS_GLIB_MESSAGE_LOOP_H_ diff --git a/brillo/message_loops/glib_message_loop_unittest.cc b/brillo/message_loops/glib_message_loop_unittest.cc deleted file mode 100644 index e9206b0..0000000 --- a/brillo/message_loops/glib_message_loop_unittest.cc +++ /dev/null @@ -1,69 +0,0 @@ -// Copyright 2015 The Chromium OS Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include <brillo/message_loops/glib_message_loop.h> - -#include <fcntl.h> -#include <unistd.h> - -#include <memory> - -#include <base/bind.h> -#include <base/location.h> -#include <base/posix/eintr_wrapper.h> -#include <gtest/gtest.h> - -#include <brillo/bind_lambda.h> -#include <brillo/message_loops/message_loop.h> -#include <brillo/message_loops/message_loop_utils.h> - -using base::Bind; - -namespace brillo { - -using TaskId = MessageLoop::TaskId; - -class GlibMessageLoopTest : public ::testing::Test { - protected: - void SetUp() override { - loop_.reset(new GlibMessageLoop()); - EXPECT_TRUE(loop_.get()); - } - - std::unique_ptr<GlibMessageLoop> loop_; -}; - -// When you watch a file descriptor for reading, the guaranties are that a -// blocking call to read() on that file descriptor will not block. This should -// include the case when the other end of a pipe is closed or the file is empty. -TEST_F(GlibMessageLoopTest, WatchFileDescriptorTriggersWhenEmpty) { - int fd = HANDLE_EINTR(open("/dev/null", O_RDONLY)); - int called = 0; - TaskId task_id = loop_->WatchFileDescriptor( - FROM_HERE, fd, MessageLoop::kWatchRead, true, - Bind([](int* called) { (*called)++; }, &called)); - EXPECT_NE(MessageLoop::kTaskIdNull, task_id); - EXPECT_NE(0, MessageLoopRunMaxIterations(loop_.get(), 10)); - EXPECT_LT(2, called); - EXPECT_TRUE(loop_->CancelTask(task_id)); -} - -// Test that an invalid file descriptor triggers the callback. -TEST_F(GlibMessageLoopTest, WatchFileDescriptorTriggersWhenInvalid) { - int fd = HANDLE_EINTR(open("/dev/zero", O_RDONLY)); - int called = 0; - TaskId task_id = loop_->WatchFileDescriptor( - FROM_HERE, fd, MessageLoop::kWatchRead, true, - Bind([] (int* called, int fd) { - if (!*called) - IGNORE_EINTR(close(fd)); - (*called)++; - }, &called, fd)); - EXPECT_NE(MessageLoop::kTaskIdNull, task_id); - EXPECT_NE(0, MessageLoopRunMaxIterations(loop_.get(), 10)); - EXPECT_LT(2, called); - EXPECT_TRUE(loop_->CancelTask(task_id)); -} - -} // namespace brillo diff --git a/brillo/message_loops/message_loop_unittest.cc b/brillo/message_loops/message_loop_unittest.cc index cf548ab..7c8d1b6 100644 --- a/brillo/message_loops/message_loop_unittest.cc +++ b/brillo/message_loops/message_loop_unittest.cc @@ -20,7 +20,6 @@ #include <brillo/bind_lambda.h> #include <brillo/unittest_utils.h> #include <brillo/message_loops/base_message_loop.h> -#include <brillo/message_loops/glib_message_loop.h> #include <brillo/message_loops/message_loop_utils.h> using base::Bind; @@ -66,11 +65,6 @@ class MessageLoopTest : public ::testing::Test { }; template <> -void MessageLoopTest<GlibMessageLoop>::MessageLoopSetUp() { - loop_.reset(new GlibMessageLoop()); -} - -template <> void MessageLoopTest<BaseMessageLoop>::MessageLoopSetUp() { base_loop_.reset(new base::MessageLoopForIO()); loop_.reset(new BaseMessageLoop(base::MessageLoopForIO::current())); @@ -78,9 +72,7 @@ void MessageLoopTest<BaseMessageLoop>::MessageLoopSetUp() { // This setups gtest to run each one of the following TYPED_TEST test cases on // on each implementation. -typedef ::testing::Types< - GlibMessageLoop, - BaseMessageLoop> MessageLoopTypes; +typedef ::testing::Types<BaseMessageLoop> MessageLoopTypes; TYPED_TEST_CASE(MessageLoopTest, MessageLoopTypes); diff --git a/libbrillo.gypi b/libbrillo.gypi index 7691c82..ed63e64 100644 --- a/libbrillo.gypi +++ b/libbrillo.gypi @@ -290,7 +290,7 @@ 'target_name': 'libbrillo-glib-<(libbase_ver)', 'type': 'shared_library', 'dependencies': [ - 'libbrillo-<(libbase_ver)', + 'libbrillo-<(libbase_ver)', ], 'variables': { 'exported_deps': [ @@ -318,9 +318,6 @@ ], }, }, - 'sources': [ - 'brillo/message_loops/glib_message_loop.cc', - ], 'includes': ['../common-mk/deps.gypi'], 'conditions': [ ['USE_dbus == 1', { @@ -340,8 +337,8 @@ 'type': 'executable', 'dependencies': [ 'libbrillo-<(libbase_ver)', - 'libbrillo-test-<(libbase_ver)', 'libbrillo-glib-<(libbase_ver)', + 'libbrillo-test-<(libbase_ver)', ], 'variables': { 'deps': [ @@ -388,7 +385,6 @@ 'brillo/map_utils_unittest.cc', 'brillo/message_loops/base_message_loop_unittest.cc', 'brillo/message_loops/fake_message_loop_unittest.cc', - 'brillo/message_loops/glib_message_loop_unittest.cc', 'brillo/message_loops/message_loop_unittest.cc', 'brillo/mime_utils_unittest.cc', 'brillo/osrelease_reader_unittest.cc', |