[mojo-edk] Expose notification source to MojoWatch callbacks

mojo/edk/system/request_context.cc

 // Copyright 2016 The Chromium 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 "mojo/edk/system/request_context.h"
 
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/threading/thread_local.h"
 
 namespace mojo {
 namespace edk {
 
 namespace {
 
 base::LazyInstance<base::ThreadLocalPointer<RequestContext>>::Leaky
     g_current_context = LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
-RequestContext::RequestContext() : tls_context_(g_current_context.Pointer()){
+RequestContext::RequestContext(Source source)
+    : source_(source), tls_context_(g_current_context.Pointer()) {
   // We allow nested RequestContexts to exist as long as they aren't actually
   // used for anything.
   if (!tls_context_->Get())

[yzshen1, 2016/03/16 21:53:52]

nit: it seems the two tls_context_->Get() (line 25 and 28) can be folded into
one.

[Ken Rockot(use gerrit already), 2016/03/17 00:15:33]

Done

     tls_context_->Set(this);
+
+  RequestContext* current_context = tls_context_->Get();

[Anand Mistry (off Chromium), 2016/03/16 23:04:14]

There's something very fishy about this code. Why do you need to save the outer
source?

There are four cases to consider (and other possible cases inductively break
down into these four):
1. LOCAL nested in LOCAL. This is the original code, and the nested context
should be ignored.
2. EXTERNAL nested in LOCAL. This should be impossible.
3. EXTERNAL nested in EXTERNAL. Should also be impossible.
4. LOCAL nested in EXTERNAL. This looks like the case you're trying to handle.
But why? Wouldn't you want to keep the existing behaviour and have the outermost
context (the external one) handle the watchers. You don't need to track the
outer context source, since that's available via TLS context at any point.

[Ken Rockot(use gerrit already), 2016/03/17 00:15:33]

I don't really know how I convinced myself that #4 was necessary to handle, but
I agree that it's not necessary. Removed this crap.

+  outer_source_ = current_context->source_;
+  if (source != outer_source_) {
+    // This should only be possible if the outer context is from an external
+    // request and the inner context is from an API call.
+    CHECK(source == Source::LOCAL_API_CALL &&
+          outer_source_ == Source::EXTERNAL_PROCESS);
+    current_context->source_ = source;
+  }
 }
 
 RequestContext::~RequestContext() {
   // NOTE: Callbacks invoked by this destructor are allowed to initiate new
   // EDK requests on this thread, so we need to reset the thread-local context
   // pointer before calling them.
+  RequestContext::Source notification_source = outer_source_;
   if (IsCurrent())
     tls_context_->Set(nullptr);
+  else
+    tls_context_->Get()->source_ = outer_source_;
+
+  MojoWatchNotificationFlags flags = MOJO_WATCH_NOTIFICATION_FLAG_NONE;
+  if (notification_source == Source::EXTERNAL_PROCESS)
+    flags |= MOJO_WATCH_NOTIFICATION_FLAG_EXTERNAL_PROCESS;
 
   for (const WatchNotifyFinalizer& watch :
       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;
-    watch.watcher->MaybeInvokeCallback(watch.result, watch.state);
+    RequestContext request_context(notification_source);
+    watch.watcher->MaybeInvokeCallback(watch.result, watch.state, flags);
   }
 
   for (const scoped_refptr<Watcher>& watcher :
           watch_cancel_finalizers_.container())
     watcher->Cancel();
 }
 
 // static
 RequestContext* RequestContext::current() {
   DCHECK(g_current_context.Pointer()->Get());
@@ skipping 22 matching lines @@
     scoped_refptr<Watcher> watcher,
     MojoResult result,
     const HandleSignalsState& state)
     : watcher(watcher), result(result), state(state) {
 }
 
 RequestContext::WatchNotifyFinalizer::~WatchNotifyFinalizer() {}
 
 }  // namespace edk
 }  // namespace mojo