Eagerly unregister PostMessageTimer as a context observer.

third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 96 matching lines @@
 
 void LocalDOMWindow::WindowFrameObserver::contextDestroyed()
 {
     m_window->frameDestroyed();
     LocalFrameLifecycleObserver::contextDestroyed();
 }
 
 class PostMessageTimer final : public NoBaseWillBeGarbageCollectedFinalized<PostMessageTimer>, public SuspendableTimer {
     WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(PostMessageTimer);
 public:
-    PostMessageTimer(LocalDOMWindow& window, PassRefPtrWillBeRawPtr<MessageEvent> event, PassRefPtrWillBeRawPtr<LocalDOMWindow> source, SecurityOrigin* targetOrigin, PassRefPtr<ScriptCallStack> stackTrace, UserGestureToken* userGestureToken)
+    PostMessageTimer(LocalDOMWindow& window, PassRefPtrWillBeRawPtr<MessageEvent> event, SecurityOrigin* targetOrigin, PassRefPtr<ScriptCallStack> stackTrace, UserGestureToken* userGestureToken)
         : SuspendableTimer(window.document())
         , m_event(event)
         , m_window(&window)
         , m_targetOrigin(targetOrigin)
         , m_stackTrace(stackTrace)
         , m_userGestureToken(userGestureToken)
-        , m_preventDestruction(false)
+        , m_disposalAllowed(true)
     {
         m_asyncOperationId = InspectorInstrumentation::traceAsyncOperationStarting(executionContext(), "postMessage");
     }
 
     PassRefPtrWillBeRawPtr<MessageEvent> event() const { return m_event.get(); }
     SecurityOrigin* targetOrigin() const { return m_targetOrigin.get(); }
     ScriptCallStack* stackTrace() const { return m_stackTrace.get(); }
     UserGestureToken* userGestureToken() const { return m_userGestureToken.get(); }
     void stop() override
     {
         SuspendableTimer::stop();
 
-        if (!m_preventDestruction) {
-            // Will destroy this object
-            m_window->removePostMessageTimer(this);
-        }
+        if (m_disposalAllowed)
+            dispose();
     }
 
     // Eager finalization is needed to promptly stop this timer object.
     // (see DOMTimer comment for more.)
     EAGERLY_FINALIZE();
     DEFINE_INLINE_VIRTUAL_TRACE()
     {
         visitor->trace(m_event);
         visitor->trace(m_window);
         SuspendableTimer::trace(visitor);
     }
 
     // TODO(alexclarke): Override timerTaskRunner() to pass in a document specific default task runner.
 
 private:
     void fired() override
     {
         InspectorInstrumentationCookie cookie = InspectorInstrumentation::traceAsyncOperationCompletedCallbackStarting(executionContext(), m_asyncOperationId);
-        // Prevent calls to stop triggered from the event handler to
-        // kill this object.
-        m_preventDestruction = true;
+        m_disposalAllowed = false;

[haraken, 2016/02/17 13:52:37]

How about moving this line into dispose()?

[sof, 2016/02/17 13:55:40]

I don't see how that's possible given that we want to neutralize event handlers
calling stop().

[haraken, 2016/02/17 13:57:19]

Makes sense.

         m_window->postMessageTimerFired(this);
-        // Will destroy this object
+        dispose();
+        InspectorInstrumentation::traceAsyncCallbackCompleted(cookie);
+    }
+
+    void dispose()
+    {
+        // Oilpan optimization: unregister as an observer right away.
+        clearContext();
+        // Will destroy this object, now or after the next GC depending
+        // on whether Oilpan is disabled or not.
         m_window->removePostMessageTimer(this);
-        InspectorInstrumentation::traceAsyncCallbackCompleted(cookie);
     }
 
     RefPtrWillBeMember<MessageEvent> m_event;
     RawPtrWillBeMember<LocalDOMWindow> m_window;
     RefPtr<SecurityOrigin> m_targetOrigin;
     RefPtr<ScriptCallStack> m_stackTrace;
     RefPtr<UserGestureToken> m_userGestureToken;
     int m_asyncOperationId;
-    bool m_preventDestruction;
+    bool m_disposalAllowed;
 };
 
 static void updateSuddenTerminationStatus(LocalDOMWindow* domWindow, bool addedListener, FrameLoaderClient::SuddenTerminationDisablerType disablerType)
 {
     Platform::current()->suddenTerminationChanged(!addedListener);
     if (domWindow->frame() && domWindow->frame()->loader().client())
         domWindow->frame()->loader().client()->suddenTerminationDisablerChanged(addedListener, disablerType);
 }
 
 using DOMWindowSet = WillBePersistentHeapHashCountedSet<RawPtrWillBeWeakMember<LocalDOMWindow>>;
@@ skipping 483 matching lines @@
     return m_applicationCache.get();
 }
 
 Navigator* LocalDOMWindow::navigator() const
 {
     if (!m_navigator)
         m_navigator = Navigator::create(frame());
     return m_navigator.get();
 }
 
-void LocalDOMWindow::schedulePostMessage(PassRefPtrWillBeRawPtr<MessageEvent> event, LocalDOMWindow* source, SecurityOrigin* target, PassRefPtr<ScriptCallStack> stackTrace)
+void LocalDOMWindow::schedulePostMessage(PassRefPtrWillBeRawPtr<MessageEvent> event, SecurityOrigin* target, PassRefPtr<ScriptCallStack> stackTrace)
 {
+    // Allowing unbounded amounts of messages to build up for a suspended context
+    // is problematic; consider imposing a limit or other restriction if this
+    // surfaces often as a problem (see crbug.com/587012).
+
     // Schedule the message.
-    OwnPtrWillBeRawPtr<PostMessageTimer> timer = adoptPtrWillBeNoop(new PostMessageTimer(*this, event, source, target, stackTrace, UserGestureIndicator::currentToken()));
+    OwnPtrWillBeRawPtr<PostMessageTimer> timer = adoptPtrWillBeNoop(new PostMessageTimer(*this, event, target, stackTrace, UserGestureIndicator::currentToken()));
     timer->startOneShot(0, BLINK_FROM_HERE);
     timer->suspendIfNeeded();
     m_postMessageTimers.add(timer.release());
 }
 
 void LocalDOMWindow::postMessageTimerFired(PostMessageTimer* timer)
 {
-    if (!isCurrentlyDisplayedInFrame()) {
+    if (!isCurrentlyDisplayedInFrame())
         return;
-    }
 
     RefPtrWillBeRawPtr<MessageEvent> event = timer->event();
 
     UserGestureIndicator gestureIndicator(timer->userGestureToken());
 
     event->entangleMessagePorts(document());
     dispatchMessageEventWithOriginCheck(timer->targetOrigin(), event, timer->stackTrace());
 }
 
 void LocalDOMWindow::removePostMessageTimer(PostMessageTimer* timer)
@@ skipping 810 matching lines @@
 {
     // If the LocalDOMWindow still has a frame reference, that frame must point
     // back to this LocalDOMWindow: otherwise, it's easy to get into a situation
     // where script execution leaks between different LocalDOMWindows.
     if (m_frameObserver->frame())
         ASSERT_WITH_SECURITY_IMPLICATION(m_frameObserver->frame()->domWindow() == this);
     return m_frameObserver->frame();
 }
 
 } // namespace blink