Make Timer and CancellableTaskFactory embeddeable in lazily swept objects.

Source/platform/scheduler/CancellableTaskFactory.cpp

 // Copyright 2015 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 "config.h"
 #include "platform/scheduler/CancellableTaskFactory.h"
 
 #include "public/platform/Platform.h"
 #include "wtf/InstanceCounter.h"
 
 namespace blink {
 
 void CancellableTaskFactory::cancel()
 {
     m_weakPtrFactory.revokeAll();
 }
 
 WebThread::Task* CancellableTaskFactory::cancelAndCreate()
 {
     cancel();
     return new CancellableTask(m_weakPtrFactory.createWeakPtr());
 }
 
+NO_LAZY_SWEEP_SANITIZE_ADDRESS
 void CancellableTaskFactory::CancellableTask::run()
 {
-    if (m_weakPtr.get()) {
-        Closure* closure = m_weakPtr->m_closure.get();
-        m_weakPtr->m_weakPtrFactory.revokeAll();
+    if (CancellableTaskFactory* taskFactory = m_weakPtr.get()) {
+#if defined(ADDRESS_SANITIZER)
+        if (taskFactory->m_unpoisonBeforeUpdate)

[haraken, 2015/06/08 06:51:52]

Do we need this condition? I mean, wouldn't it be better to unpoison the memory
unconditionally? For example, this CL takes care of CancellableTaskFactory in
Timer but doesn't take care of CancellableTaskFactory in ScriptRunner. It seems
that what we want here is:

- Mark CancellableTaskFactory DISALLOW_ALLOCATION()
- Unconditionally unpoison the memory here.

[sof, 2015/06/08 07:04:37]

If you do it that way, you will also unpoison non-heap uses of Timer(Base) et
al, which I wanted to avoid. CancelleableTaskFactory cannot tidily(?) determine
if it is appearing on the heap.

[haraken, 2015/06/08 07:11:10]

ah, makes sense. The current CL looks good :)

+            ASAN_UNPOISON_MEMORY_REGION(reinterpret_cast<unsigned char*>(taskFactory), sizeof(CancellableTaskFactory));

[haraken, 2015/06/06 15:25:20]

Just help me understand: Where can the CancellableTaskFactory get accessed after
it's poisoned? I'm curious why it's hard to circumvent the errors with
NO_LAZY_SWEEP_SANITIZE_ADDRESS.

It looks like that CancellableTaskFactory is used only by HTMLParserScheduler
and ScriptRunner. Maybe can we just change them to use
OwnPtr<CancellableTaskFactory>s?

[sof, 2015/06/06 15:46:11]

Hmm, it's accessed from right here?

Timer's use of it is the one to be concerned with.

[haraken, 2015/06/07 16:40:46]

This function has NO_LAZY_SWEEP_SANITIZE_ADDRESS. I'm wondering why it isn't
enough to circumvent the poisoning error?

[sof, 2015/06/07 17:23:05]

That's a leaf-level annotation. OwnPtr::get() is not covered, nor is
revokeAll().

[haraken, 2015/06/07 23:46:24]

Thanks, understood.

So the final question would be: isn't there any option to make
HTMLParserScheduler and ScriptRunner hold an OwnPtr<CancellableTaskFactory>?
Then the CancellableTaskFactory won't be poisoned and thus the problem will be
gone.

+#endif
+        Closure* closure = taskFactory->m_closure.get();
+        taskFactory->m_weakPtrFactory.revokeAll();
         (*closure)();
     }
 }
 
 } // namespace blink