[Synchronization] Fix a crash in WaitableEventWatcher

base/synchronization/waitable_event_watcher.h

Index: base/synchronization/waitable_event_watcher.h
diff --git a/base/synchronization/waitable_event_watcher.h b/base/synchronization/waitable_event_watcher.h
index 4c83e21b289c7e2634a18f954cb635edd31f5357..4b88d3d0dd49ccfe423e8bae164bcce90df038d2 100644
--- a/base/synchronization/waitable_event_watcher.h
+++ b/base/synchronization/waitable_event_watcher.h
@@ -7,13 +7,14 @@
 
 #include "base/base_export.h"
 #include "base/macros.h"
-#include "base/sequence_checker.h"
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
 #include "base/win/object_watcher.h"
+#include "base/win/scoped_handle.h"
 #else
 #include "base/callback.h"
+#include "base/sequence_checker.h"
 #include "base/synchronization/waitable_event.h"
 #endif
 
@@ -56,7 +57,8 @@ class WaitableEvent;
 // missing a signal.
 //
 // NOTE: you /are/ allowed to delete the WaitableEvent while still waiting on
-// it with a Watcher. It will act as if the event was never signaled.
+// it with a Watcher. But pay attention: if the event was signaled and deleted

[danakj, 2017/04/27 20:01:08]

ISTM that all WaitableEventWatchers' EventCallbacks are only using the
WaitableEvent* passed to it for pointer comparison. This is good as it could be
deleted as this comment says. What do you think of changing it to a void*?

[atuchin, 2017/04/28 06:44:29]

As for me it is better to remove that param at all.
I looked on several use cases and found only DCHECKs.

I chose to not modify WaitableEventWatcher interface in that CL (it isn't
related with the crash).
Can you create bug/task to do that?

[danakj, 2017/04/28 17:54:49]

I would like to do it together because the pointer was safe to use before but
now it's not, if you think its reasonable. Keeping the DCHECKs doesn't seem
harmful and they'd work with a void* too.

[atuchin, 2017/05/02 07:41:19]

Unfortunately, the pointer wasn't safe before.
As you can see I don't change the behaviour of WaitableEventWatcher.
I change only watching handle. 

And using void* here doesn't look a good solution for me because after deleting
such pointers are not unique
(a new WaitableEvent may have the same address). That why I would like to not
change it here.

+// right after, the callback may be called with deleted WaitableEvent pointer.
 
 class BASE_EXPORT WaitableEventWatcher
 #if defined(OS_WIN)
@@ -90,6 +92,7 @@ class BASE_EXPORT WaitableEventWatcher
 #if defined(OS_WIN)
   void OnObjectSignaled(HANDLE h) override;
 
+  win::ScopedHandle event_handle_;
   win::ObjectWatcher watcher_;
   EventCallback callback_;
   WaitableEvent* event_ = nullptr;