Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(511)

Unified Diff: mojo/edk/system/request_context.cc

Issue 2750373002: Revert of Mojo: Armed Watchers (Closed)
Patch Set: Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « mojo/edk/system/request_context.h ('k') | mojo/edk/system/watch.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: mojo/edk/system/request_context.cc
diff --git a/mojo/edk/system/request_context.cc b/mojo/edk/system/request_context.cc
index 5de65d7b641521fa64ea00e57257c5ae12802fcc..6370ab11c77f4464d4caf8226b4e6209867721e7 100644
--- a/mojo/edk/system/request_context.cc
+++ b/mojo/edk/system/request_context.cc
@@ -36,34 +36,27 @@
// since we're starting over at the bottom of the stack.
tls_context_->Set(nullptr);
- MojoWatcherNotificationFlags flags = MOJO_WATCHER_NOTIFICATION_FLAG_NONE;
+ MojoWatchNotificationFlags flags = MOJO_WATCH_NOTIFICATION_FLAG_NONE;
if (source_ == Source::SYSTEM)
- flags |= MOJO_WATCHER_NOTIFICATION_FLAG_FROM_SYSTEM;
+ flags |= MOJO_WATCH_NOTIFICATION_FLAG_FROM_SYSTEM;
- // We send all cancellation notifications first. This is necessary because
- // it's possible that cancelled watches have other pending notifications
+ // We run all cancellation finalizers first. This is necessary because it's
+ // possible that one of the cancelled watchers has other pending finalizers
// attached to this RequestContext.
//
- // From the application's perspective the watch is cancelled as soon as this
- // notification is received, and dispatching the cancellation notification
- // updates some internal Watch state to ensure no further notifications
- // fire. Because notifications on a single Watch are mutually exclusive,
- // this is sufficient to guarantee that MOJO_RESULT_CANCELLED is the last
- // notification received; which is the guarantee the API makes.
- for (const scoped_refptr<Watch>& watch :
- watch_cancel_finalizers_.container()) {
- static const HandleSignalsState closed_state = {0, 0};
-
- // Establish a new RequestContext to capture and run any new notifications
- // triggered by the callback invocation.
- RequestContext inner_context(source_);
- watch->InvokeCallback(MOJO_RESULT_CANCELLED, closed_state, flags);
- }
+ // From the application's perspective the watch has already been cancelled,
+ // so we have to honor our contract which guarantees no more notifications.
+ for (const scoped_refptr<Watcher>& watcher :
+ watch_cancel_finalizers_.container())
+ watcher->Cancel();
for (const WatchNotifyFinalizer& watch :
- watch_notify_finalizers_.container()) {
- RequestContext inner_context(source_);
- watch.watch->InvokeCallback(watch.result, watch.state, flags);
+ watch_notify_finalizers_.container()) {
+ // Establish a new request context for the extent of each callback to
+ // ensure that they don't themselves invoke callbacks while holding a
+ // watcher lock.
+ RequestContext request_context(source_);
+ watch.watcher->MaybeInvokeCallback(watch.result, watch.state, flags);
}
} else {
// It should be impossible for nested contexts to have finalizers.
@@ -78,17 +71,18 @@
return g_current_context.Pointer()->Get();
}
-void RequestContext::AddWatchNotifyFinalizer(scoped_refptr<Watch> watch,
- MojoResult result,
- const HandleSignalsState& state) {
+void RequestContext::AddWatchNotifyFinalizer(
+ scoped_refptr<Watcher> watcher,
+ MojoResult result,
+ const HandleSignalsState& state) {
DCHECK(IsCurrent());
watch_notify_finalizers_->push_back(
- WatchNotifyFinalizer(std::move(watch), result, state));
+ WatchNotifyFinalizer(std::move(watcher), result, state));
}
-void RequestContext::AddWatchCancelFinalizer(scoped_refptr<Watch> watch) {
+void RequestContext::AddWatchCancelFinalizer(scoped_refptr<Watcher> watcher) {
DCHECK(IsCurrent());
- watch_cancel_finalizers_->push_back(std::move(watch));
+ watch_cancel_finalizers_->push_back(std::move(watcher));
}
bool RequestContext::IsCurrent() const {
@@ -96,10 +90,10 @@
}
RequestContext::WatchNotifyFinalizer::WatchNotifyFinalizer(
- scoped_refptr<Watch> watch,
+ scoped_refptr<Watcher> watcher,
MojoResult result,
const HandleSignalsState& state)
- : watch(std::move(watch)), result(result), state(state) {}
+ : watcher(std::move(watcher)), result(result), state(state) {}
RequestContext::WatchNotifyFinalizer::WatchNotifyFinalizer(
const WatchNotifyFinalizer& other) = default;
« no previous file with comments | « mojo/edk/system/request_context.h ('k') | mojo/edk/system/watch.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698