Revert of Mojo: Armed Watchers

Revert of Mojo: Armed Watchers (patchset #20 id:670001 of https://codereview.chromium.org/2725133002/ )

Reason for revert:
FindIt considers this as culprit of breaking  Linux Chromium OS ASan LSan Tests (1) (stats) bot
See
https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/20033

aura_unittests aura_unittests

failed 1
Run on OS: 'Ubuntu-14.04'
failures:
UserActivityForwarderTest.ForwardActivityToDetector


Will reland if the revert doesn't fix the bot.


Original issue's description:
> Mojo: Armed Watchers
> 
> Changes the Watcher C API as described in this post:
> https://groups.google.com/a/chromium.org/forum/#!topic/chromium-mojo/UcA97R4IznI
> 
> Makes watchers a first-class non-transferable handle type in Mojo,
> allows multiple distinct events to be watched by a single watcher,
> and requires watchers to be explicitly armed before a notification
> will fire. Also now allows for cancellation (and watcher closure)
> from within notification callbacks, simplifying cancellation logic
> and avoiding any possibility of EDK deadlocks caused by cancellations,
> during watch notification.
> 
> Updates the Watcher class in mojo/public/cpp/system to reflect the
> new model, adding an explicit ArmingPolicy to allow users to select
> manual or automatic arming. Also renames it to SimpleWatcher to
> adequately convey that this is only a simplified helper class that
> does not utilize the full power of watchers.
> 
> Automatic arming provides imperfect edge-triggered behavior, which is
> still an improvement over the old behavior in many cases.
> 
> Manual arming is used in the bindings Connector to ensure that all
> messages are flushed from a pipe before control returns from a
> handle-ready notification, and is also now used for Watchers which
> watch a data pipe handle.
> 
> Other users of the Watcher C API (namely Blink's MojoWatcher and
> content's MessagePort) have also been adapted to the new API.
> 
> BUG=693595, 700171
> 
> Review-Url: https://codereview.chromium.org/2725133002
> Cr-Commit-Position: refs/heads/master@{#457269}
> Committed: https://chromium.googlesource.com/chromium/src/+/9eadabae1de0cb22725450596fe324f03dd63b92

TBR=csharrison@chromium.org,eugenebut@chromium.org,haraken@chromium.org,jam@chromium.org,miu@chromium.org,mmenke@chromium.org,xhwang@chromium.org,yhirano@chromium.org,yzshen@chromium.org,rockot@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=693595, 700171

[Guido Urdaneta, 2017-03-16 15:04:33]

The CQ bit was checked by guidou@chromium.org

[Guido Urdaneta, 2017-03-16 15:04:34]

Created Revert of Mojo: Armed Watchers

[commit-bot: I haz the power, 2017-03-16 15:04:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-16 15:07:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-16 15:07:15]

Failed to apply patch for mojo/edk/system/awakable_list.h:
While running git apply --index -p1;
  error: patch failed: mojo/edk/system/awakable_list.h:12
  error: mojo/edk/system/awakable_list.h: patch does not apply

Patch:       mojo/edk/system/awakable_list.h
Index: mojo/edk/system/awakable_list.h
diff --git a/mojo/edk/system/awakable_list.h b/mojo/edk/system/awakable_list.h
index
34d6b06922a041498d06a7d155c1124f4a8922d7..355677f7036771af72221e0ceee8d9d7c09b6f60
100644
--- a/mojo/edk/system/awakable_list.h
+++ b/mojo/edk/system/awakable_list.h
@@ -12,6 +12,8 @@
 
 #include "base/macros.h"
 #include "mojo/edk/system/system_impl_export.h"
+#include "mojo/edk/system/watcher.h"
+#include "mojo/edk/system/watcher_set.h"
 #include "mojo/public/c/system/types.h"
 
 namespace mojo {
@@ -37,6 +39,13 @@
   void Add(Awakable* awakable, MojoHandleSignals signals, uintptr_t context);
   void Remove(Awakable* awakable);
 
+  // Add and remove Watchers to this AwakableList.
+  MojoResult AddWatcher(MojoHandleSignals signals,
+                        const Watcher::WatchCallback& callback,
+                        uintptr_t context,
+                        const HandleSignalsState& current_state);
+  MojoResult RemoveWatcher(uintptr_t context);
+
  private:
   struct AwakeInfo {
     AwakeInfo(Awakable* awakable, MojoHandleSignals signals, uintptr_t context)
@@ -50,6 +59,10 @@
 
   AwakeInfoList awakables_;
 
+  // TODO: Remove AwakableList and instead use WatcherSet directly in
+  // dispatchers.
+  WatcherSet watchers_;
+
   DISALLOW_COPY_AND_ASSIGN(AwakableList);
 };

[Guido Urdaneta, 2017-03-16 15:14:47]

The CQ bit was unchecked by guidou@chromium.org

[Guido Urdaneta, 2017-03-16 15:15:23]

Cancelled the revert since rockot@ claims to have a fix.

[Guido Urdaneta, 2017-03-16 15:27:27]

Description was changed from

==========
Revert of Mojo: Armed Watchers (patchset #20 id:670001 of
https://codereview.chromium.org/2725133002/ )

Reason for revert:
FindIt considers this as culprit of breaking  Linux Chromium OS ASan LSan Tests
(1) (stats) bot
See
https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%2...

aura_unittests aura_unittests

failed 1
Run on OS: 'Ubuntu-14.04'
failures:
UserActivityForwarderTest.ForwardActivityToDetector


Will reland if the revert doesn't fix the bot.


Original issue's description:
> Mojo: Armed Watchers
> 
> Changes the Watcher C API as described in this post:
>
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-mojo/UcA97R4IznI
> 
> Makes watchers a first-class non-transferable handle type in Mojo,
> allows multiple distinct events to be watched by a single watcher,
> and requires watchers to be explicitly armed before a notification
> will fire. Also now allows for cancellation (and watcher closure)
> from within notification callbacks, simplifying cancellation logic
> and avoiding any possibility of EDK deadlocks caused by cancellations,
> during watch notification.
> 
> Updates the Watcher class in mojo/public/cpp/system to reflect the
> new model, adding an explicit ArmingPolicy to allow users to select
> manual or automatic arming. Also renames it to SimpleWatcher to
> adequately convey that this is only a simplified helper class that
> does not utilize the full power of watchers.
> 
> Automatic arming provides imperfect edge-triggered behavior, which is
> still an improvement over the old behavior in many cases.
> 
> Manual arming is used in the bindings Connector to ensure that all
> messages are flushed from a pipe before control returns from a
> handle-ready notification, and is also now used for Watchers which
> watch a data pipe handle.
> 
> Other users of the Watcher C API (namely Blink's MojoWatcher and
> content's MessagePort) have also been adapted to the new API.
> 
> BUG=693595,700171
> 
> Review-Url: https://codereview.chromium.org/2725133002
> Cr-Commit-Position: refs/heads/master@{#457269}
> Committed:
https://chromium.googlesource.com/chromium/src/+/9eadabae1de0cb22725450596fe3...

TBR=csharrison@chromium.org,eugenebut@chromium.org,haraken@chromium.org,jam@c...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=693595,700171
==========

to

==========
Revert of Mojo: Armed Watchers (patchset #20 id:670001 of
https://codereview.chromium.org/2725133002/ )

Reason for revert:
FindIt considers this as culprit of breaking  Linux Chromium OS ASan LSan Tests
(1) (stats) bot
See
https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%2...

aura_unittests aura_unittests

failed 1
Run on OS: 'Ubuntu-14.04'
failures:
UserActivityForwarderTest.ForwardActivityToDetector


Will reland if the revert doesn't fix the bot.


Original issue's description:
> Mojo: Armed Watchers
> 
> Changes the Watcher C API as described in this post:
>
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-mojo/UcA97R4IznI
> 
> Makes watchers a first-class non-transferable handle type in Mojo,
> allows multiple distinct events to be watched by a single watcher,
> and requires watchers to be explicitly armed before a notification
> will fire. Also now allows for cancellation (and watcher closure)
> from within notification callbacks, simplifying cancellation logic
> and avoiding any possibility of EDK deadlocks caused by cancellations,
> during watch notification.
> 
> Updates the Watcher class in mojo/public/cpp/system to reflect the
> new model, adding an explicit ArmingPolicy to allow users to select
> manual or automatic arming. Also renames it to SimpleWatcher to
> adequately convey that this is only a simplified helper class that
> does not utilize the full power of watchers.
> 
> Automatic arming provides imperfect edge-triggered behavior, which is
> still an improvement over the old behavior in many cases.
> 
> Manual arming is used in the bindings Connector to ensure that all
> messages are flushed from a pipe before control returns from a
> handle-ready notification, and is also now used for Watchers which
> watch a data pipe handle.
> 
> Other users of the Watcher C API (namely Blink's MojoWatcher and
> content's MessagePort) have also been adapted to the new API.
> 
> BUG=693595,700171
> 
> Review-Url: https://codereview.chromium.org/2725133002
> Cr-Commit-Position: refs/heads/master@{#457269}
> Committed:
https://chromium.googlesource.com/chromium/src/+/9eadabae1de0cb22725450596fe3...

TBR=csharrison@chromium.org,eugenebut@chromium.org,haraken@chromium.org,jam@c...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=693595,700171
==========