Bug 165633 – Web Inspector: Need some limit on Async Call Stacks for async loops (rAF loops)

• Component: Web Inspector
• Status: RESOLVED FIXED

[Joseph Pecoraro]

Created attachment 296579 [details]
[IMAGE] Large Async Call Stacks Due to Async Loop

Summary:
Need some limit on Async Call Stacks for async loops (rAF loops)

For rAF loops the Async Call Stacks get too large. This produces a difficult to use / understand UI.

Steps to Reproduce:
1. Inspect http://tesseract.projectnaptha.com
2. Wait a few seconds
3. Click Debugger Pause button
  => Main Thread has a very very large call stack with rAFs

[Radar WebKit Bug Importer]

<rdar://problem/29738502>

[Matt Baker]

Created attachment 298525 [details]
[Image] Proposed UI

The UI should have some kind of indication that the call stack has been truncated. Xcode indicates the presence of elided call frames with a dashed separator (and a gap in the call frame indices).

The scenario for the Inspector is somewhat different, but we can take a cue from Xcode's UI. The attached screenshot shows a truncated call stack*, and two proposals for indicating that frames have been dropped.

* Stack trace limit was set to a small number to get the desired output

[Matt Baker]

(In reply to comment #2)
> The scenario for the Inspector is somewhat different, but we can take a cue
> from Xcode's UI. The attached screenshot shows a truncated call stack*, and
> two proposals for indicating that frames have been dropped.

I'm leaning toward the dimmed function icon.

[Joseph Pecoraro]

(In reply to comment #3)
> (In reply to comment #2)
> > The scenario for the Inspector is somewhat different, but we can take a cue
> > from Xcode's UI. The attached screenshot shows a truncated call stack*, and
> > two proposals for indicating that frames have been dropped.
> 
> I'm leaning toward the dimmed function icon.

Doesn't that already mean something for Tail Deleted Frames?

[Matt Baker]

(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > The scenario for the Inspector is somewhat different, but we can take a cue
> > > from Xcode's UI. The attached screenshot shows a truncated call stack*, and
> > > two proposals for indicating that frames have been dropped.
> > 
> > I'm leaning toward the dimmed function icon.
> 
> Doesn't that already mean something for Tail Deleted Frames?

Tail deleted frames use a gray function icon (TailDeletedFunction.svg).

[Blaze Burg]

It looks strange that the async horizontal rule goes all the way across but the elided frames rule is indented. Can you make it go the full width too? How do either of the rules look when we have worker contexts in the sidebar?

I would prefer the UIString "(Earlier frames unavailable)" or something. 'Removed' seems like the wrong word. Truncated or Elided might be okay.

[Matt Baker]

Created attachment 299794 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 299794 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=299794&action=review

> Source/JavaScriptCore/ChangeLog:30
> +        A node is either `pending`, `active`, `dispatched`, or `canceled`, and
> +        all nodes start out as pending. After a callback for a pending node is

Split into two sentences at the "and".

> Source/JavaScriptCore/ChangeLog:32
> +        (e.g. setInterval), in which case it remains pending. Once a nodes is no

Typo: "a nodes" => "a node"

> Source/JavaScriptCore/ChangeLog:62
> +        * JavaScriptCore.xcodeproj/project.pbxproj:
> +        * inspector/AsyncStackTrace.cpp: Added.

The new file needs to be added to CMakeLists.txt

> Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:8811
>  				0F0B839D14BCF46600885B4F /* LLIntThunks.h in Headers */,
> +				6A38CFAA1E32B5AB0060206F /* AsyncStackTrace.h in Headers */,
>  				142E3139134FF0A600AFADB5 /* Local.h in Headers */,

You should probably run:

    ./Tools/Scripts/sort-Xcode-project-file Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

We should all get in the habit of doing that more often.

> Source/JavaScriptCore/inspector/AsyncStackTrace.cpp:47
> +}

We should add a destructor with: ASSERT(!m_childCount)

> Source/JavaScriptCore/inspector/AsyncStackTrace.cpp:69
> +    ASSERT(m_state != State::Dispatched);

I think a better ASSERT would be ASSERT(Active || Canceled). I don't think we can be Pending or Dispatched here.

> Source/JavaScriptCore/inspector/AsyncStackTrace.cpp:74
> +    if (!m_singleShot) {
> +        m_state = State::Pending;
> +        return;
> +    }

This should not reset to Pending if the current state is Canceled.

> Source/JavaScriptCore/inspector/AsyncStackTrace.cpp:84
> +    ASSERT(m_state != State::Canceled);

I'm not sure you can assert this.

What happens in this test case:

    let id = setTimeout(() => {
        clearTimeout(id);
        clearTimeout(id);
    });

I think we would get here twice. The first will cancel and the second will get here when cancelled.

> Source/JavaScriptCore/inspector/AsyncStackTrace.cpp:161
> +        if (!parentNode->m_parent) {

You don't need to clone up the entire tree here. You only need to clone up until maxDepth, this may be cloning more than is necessary.

> Source/JavaScriptCore/inspector/AsyncStackTrace.cpp:166
> +        currentNode->m_parent = AsyncStackTrace::create(parentNode->m_callStack, true, parentNode->m_parent);

We should be decrementing the m_childCount of the first locked ancestor because we are creating a parallel tree. I know I can come up with a bad scenario if you want me to.

> Source/JavaScriptCore/inspector/AsyncStackTrace.cpp:182
> +    m_parent = nullptr;

Am I understanding this correctly: as soon as this happens an entire line of parents with 1 ref would reach 0 refs and be destructed? So in the loop above, m_childCount accounting may make a bunch of nodes get to zero, and this line causes a chain dealloc of them all?

> Source/JavaScriptCore/inspector/AsyncStackTrace.h:61
> +    bool isTruncated() const;

This has no implementation, you can remove.

> Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp:265
> +    if (m_currentAsyncCallIdentifier && m_currentAsyncCallIdentifier.value() == identifier)
> +        return;

So if I understand this correctly.

    - setTimeout, cancelTimeout => not current will remove
    - setTimeout, willDispatch, cancelTimeout => current will not remove => didDispatch => remove
    - setInterval, cancelInterval => not active will remove
    - setInterval, willDispatch, didDispatch => not single shot so will not remove
    - setInterval, willDispatch, cancelInterval => current will not remove => didDispatch => should remove (BUG)

The last case has an issue:

    - setInterval
        - Creates AsyncStackTrace (Interval, 1234) => Pending
    - willDispatch
        - willDispatchAsyncCall Updates AsyncStackTrace (Interval, 1234) => Active
        - Current (Interval, 1234)
    - cancelInterval
        - didCancelAsyncCall Updates AsyncStackTrace (Interval, 1234) => Canceled
        - Not removed since it is current
    - didDispatch
        - didDispatchAsyncCall Updates AsyncStackTrace (Interval, 1234) => Pending
        - Not removed since it is pending, but it should have been removed

Looks like didDispatchAsyncCall should NOT reset the state from Canceled to Pending. That would leak until page navigation or inspector closes.

> Source/JavaScriptCore/inspector/protocol/Console.json:43
> +                { "name": "truncated", "type": "boolean", "optional": true, "description": "Whether one or more frames have been truncated from the bottom of the stack." },

r- because there needs to be a test for protocol changes.

Also, I think you will also need to include this in:

    Source/WebInspectorUI/Versions/Inspector-iOS-10.3.json

And rebuild the backend commands for it:

    Source/WebInspectorUI/Scripts/update-LegacyInspectorBackendCommands.rb

> Source/WebInspectorUI/UserInterface/Views/ThreadTreeElement.css:56
> +    border-top: dashed 1px var(--border-color);

I think this should be 0.5px (hairline) for Retina. I believe that is our usual style.

> Source/WebInspectorUI/UserInterface/Views/ThreadTreeElement.css:64
> +    margin-left: 0;

I'm surprised this would be needed.

[Matt Baker]

Comment on attachment 299794 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=299794&action=review

>> Source/JavaScriptCore/inspector/AsyncStackTrace.cpp:161
>> +        if (!parentNode->m_parent) {
> 
> You don't need to clone up the entire tree here. You only need to clone up until maxDepth, this may be cloning more than is necessary.

Since lastUnlockedAncestor != newStackTraceRoot we should break out once the next parent is the new root. Setting the truncate flag, etc, can be done outside the loop.

>> Source/JavaScriptCore/inspector/AsyncStackTrace.cpp:182
>> +    m_parent = nullptr;
> 
> Am I understanding this correctly: as soon as this happens an entire line of parents with 1 ref would reach 0 refs and be destructed? So in the loop above, m_childCount accounting may make a bunch of nodes get to zero, and this line causes a chain dealloc of them all?

Correct! Child counts must be updated by hand, but branches without a pending descendent clean up after themselves.

>> Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp:265
>> +        return;
> 
> So if I understand this correctly.
> 
>     - setTimeout, cancelTimeout => not current will remove
>     - setTimeout, willDispatch, cancelTimeout => current will not remove => didDispatch => remove
>     - setInterval, cancelInterval => not active will remove
>     - setInterval, willDispatch, didDispatch => not single shot so will not remove
>     - setInterval, willDispatch, cancelInterval => current will not remove => didDispatch => should remove (BUG)
> 
> The last case has an issue:
> 
>     - setInterval
>         - Creates AsyncStackTrace (Interval, 1234) => Pending
>     - willDispatch
>         - willDispatchAsyncCall Updates AsyncStackTrace (Interval, 1234) => Active
>         - Current (Interval, 1234)
>     - cancelInterval
>         - didCancelAsyncCall Updates AsyncStackTrace (Interval, 1234) => Canceled
>         - Not removed since it is current
>     - didDispatch
>         - didDispatchAsyncCall Updates AsyncStackTrace (Interval, 1234) => Pending
>         - Not removed since it is pending, but it should have been removed
> 
> Looks like didDispatchAsyncCall should NOT reset the state from Canceled to Pending. That would leak until page navigation or inspector closes.

Great catch!

>> Source/JavaScriptCore/inspector/protocol/Console.json:43
>> +                { "name": "truncated", "type": "boolean", "optional": true, "description": "Whether one or more frames have been truncated from the bottom of the stack." },
> 
> r- because there needs to be a test for protocol changes.
> 
> Also, I think you will also need to include this in:
> 
>     Source/WebInspectorUI/Versions/Inspector-iOS-10.3.json
> 
> And rebuild the backend commands for it:
> 
>     Source/WebInspectorUI/Scripts/update-LegacyInspectorBackendCommands.rb

The only protocol change was the addition of the truncated property to Console.StackTrace, which doesn't impact InspectorBackendCommands.

[Joseph Pecoraro]

Comment on attachment 299794 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=299794&action=review

>>> Source/JavaScriptCore/inspector/protocol/Console.json:43
>>> +                { "name": "truncated", "type": "boolean", "optional": true, "description": "Whether one or more frames have been truncated from the bottom of the stack." },
>> 
>> r- because there needs to be a test for protocol changes.
>> 
>> Also, I think you will also need to include this in:
>> 
>>     Source/WebInspectorUI/Versions/Inspector-iOS-10.3.json
>> 
>> And rebuild the backend commands for it:
>> 
>>     Source/WebInspectorUI/Scripts/update-LegacyInspectorBackendCommands.rb
> 
> The only protocol change was the addition of the truncated property to Console.StackTrace, which doesn't impact InspectorBackendCommands.

Any change to a protocol json file modifies an InspectorBackendCommands.

Your change here modifies the generated InspectorBackendCommands, which gets copied into UserInterface at build time.

I'm saying that you will need to edit the Inspector-iOS-10.3.json and when you do you will need to update its generated UserInterface/Protocol/Legacy/10.3/InspectorBackendCommands.js. The above script will do that.

[Matt Baker]

Created attachment 299968 [details]
Patch

[Build Bot]

Comment on attachment 299968 [details]
Patch

Attachment 299968 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2960532

New failing tests:
inspector/debugger/breakpoint-action-eval.html

[Build Bot]

Created attachment 299979 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Matt Baker]

Fixing!

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x000000010db35227 WTFCrash + 39 (Assertions.cpp:323)
1   com.apple.JavaScriptCore      	0x000000010caffd09 Inspector::AsyncStackTrace::~AsyncStackTrace() + 73 (AsyncStackTrace.cpp:51)
2   com.apple.JavaScriptCore      	0x000000010caffd65 Inspector::AsyncStackTrace::~AsyncStackTrace() + 21 (AsyncStackTrace.cpp:52)
3   com.apple.JavaScriptCore      	0x000000010cb043d9 WTF::RefCounted<Inspector::AsyncStackTrace>::deref() const + 73 (RefCounted.h:145)
4   com.apple.JavaScriptCore      	0x000000010cb04384 void WTF::derefIfNotNull<Inspector::AsyncStackTrace>(Inspector::AsyncStackTrace*) + 52 (PassRefPtr.h:41)
...

[Matt Baker]

(In reply to comment #14)
> Fixing!
> 
> Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
> 0   com.apple.JavaScriptCore      	0x000000010db35227 WTFCrash + 39
> (Assertions.cpp:323)
> 1   com.apple.JavaScriptCore      	0x000000010caffd09
> Inspector::AsyncStackTrace::~AsyncStackTrace() + 73 (AsyncStackTrace.cpp:51)
> 2   com.apple.JavaScriptCore      	0x000000010caffd65
> Inspector::AsyncStackTrace::~AsyncStackTrace() + 21 (AsyncStackTrace.cpp:52)
> 3   com.apple.JavaScriptCore      	0x000000010cb043d9
> WTF::RefCounted<Inspector::AsyncStackTrace>::deref() const + 73
> (RefCounted.h:145)
> 4   com.apple.JavaScriptCore      	0x000000010cb04384 void
> WTF::derefIfNotNull<Inspector::AsyncStackTrace>(Inspector::AsyncStackTrace*)
> + 52 (PassRefPtr.h:41)
> ...

It seems like this could be the result of InspectorDebuggerAgent tearing down a non-empty pending stack traces map.

[Matt Baker]

Created attachment 299983 [details]
Patch

[Build Bot]

Comment on attachment 299983 [details]
Patch

Attachment 299983 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2961036

New failing tests:
inspector/debugger/breakpoint-action-eval.html

[Build Bot]

Created attachment 299989 [details]
Archive of layout-test-results from ews113 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Matt Baker]

(In reply to comment #18)
> Created attachment 299989 [details]
> Archive of layout-test-results from ews113 for mac-elcapitan
> 
> The attached test failures were seen while running run-webkit-tests on the
> mac-debug-ews.
> Bot: ews113  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

🤔

[Matt Baker]

Created attachment 299999 [details]
Patch

[Build Bot]

Comment on attachment 299999 [details]
Patch

Attachment 299999 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2962295

New failing tests:
inspector/debugger/breakpoint-action-eval.html

[Build Bot]

Created attachment 300006 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Joseph Pecoraro]

Comment on attachment 299999 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=299999&action=review

r=me

> Source/JavaScriptCore/inspector/AsyncStackTrace.cpp:177
> +    // Decrement the child count of the first locked ancestor now that itafter removing its subtree.

Typo: "now that itafter".

> Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp:1144
> +    for (auto& entry : m_pendingAsyncCalls) {
> +        auto& stackTrace = entry.value;
> +        if (stackTrace->isPending())
> +            stackTrace->didCancelAsyncCall();
> +    }

I feel like this is unnecessary. With m_pendingAsyncCalls.clear() all of the "leaf" nodes will be dereferenced, causing all of their chains to be dereferenced. Why do we care to cancel things? (I could see maybe doing this if there were some Assertions in an ~AsyncStackTrace but that doesn't exist).

> LayoutTests/inspector/debugger/async-stack-trace-expected.txt:82
> +Set Debugger.asyncStackTraceDepth equal to 4
> +PASS: Non-root StackTrace should not be truncated.
> +PASS: Non-root StackTrace should not be truncated.
> +PASS: StackTrace root should be truncated.
> +PASS: StackTrace should be truncated to the nearest call stack.
> +0: [F] pauseThenFinishTest
> +1: [F] repeat
> +2: --- requestAnimationFrame ---
> +3: [F] repeat
> +4: --- requestAnimationFrame ---
> +5: [F] repeat
> +6: --- truncated ---

Interesting. This feels off-by-one, but I don't mind that much. The depth is 4, but we have (1) pauseThenFinishTest, (2) repeat, (3) rAF, (4) repeat, (5) rAF, (6) repeat, (7) truncated. Seems we could have stopped here at frame (5).

[Joseph Pecoraro]

Comment on attachment 299999 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=299999&action=review

> Source/JavaScriptCore/inspector/AsyncStackTrace.cpp:52
> +AsyncStackTrace::~AsyncStackTrace()
> +{
> +    ASSERT(!m_childCount);
> +}

Wow I totally missed the constructor despite looking for it... Yes this ASSERT is triggering because of:

18  com.apple.JavaScriptCore      	0x0000000105ad54de Inspector::InspectorDebuggerAgent::clearAsyncStackTraceData() + 270 (InspectorDebuggerAgent.cpp:1147)

And that makes sense, we are deferring the leaf nodes in a random order, and we are not removing them. Which is fine because we really do just want to release all the data and we don't need to update the internal numbers.

But if you want to keep debug code, you could probably do:

#ifndef NDEBUG
    if (m_parent)
        remove();
    ASSERT(!m_childCount);
#endif

Which I think may work and could catch other accounting issues. But it might not be worth it.

What do you think?

[Matt Baker]

Comment on attachment 299999 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=299999&action=review

>> LayoutTests/inspector/debugger/async-stack-trace-expected.txt:82
>> +6: --- truncated ---
> 
> Interesting. This feels off-by-one, but I don't mind that much. The depth is 4, but we have (1) pauseThenFinishTest, (2) repeat, (3) rAF, (4) repeat, (5) rAF, (6) repeat, (7) truncated. Seems we could have stopped here at frame (5).

1) The first two frames belong to the non-asynchronous (live) call stack, and don't count toward the async stack depth budget.
2) Frame 6 is just an indicated that the trace was truncated, not a frame.

I'll improve the call stack logging to make this clear.

[Matt Baker]

Comment on attachment 299999 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=299999&action=review

>> Source/JavaScriptCore/inspector/AsyncStackTrace.cpp:52
>> +}
> 
> Wow I totally missed the constructor despite looking for it... Yes this ASSERT is triggering because of:
> 
> 18  com.apple.JavaScriptCore      	0x0000000105ad54de Inspector::InspectorDebuggerAgent::clearAsyncStackTraceData() + 270 (InspectorDebuggerAgent.cpp:1147)
> 
> And that makes sense, we are deferring the leaf nodes in a random order, and we are not removing them. Which is fine because we really do just want to release all the data and we don't need to update the internal numbers.
> 
> But if you want to keep debug code, you could probably do:
> 
> #ifndef NDEBUG
>     if (m_parent)
>         remove();
>     ASSERT(!m_childCount);
> #endif
> 
> Which I think may work and could catch other accounting issues. But it might not be worth it.
> 
> What do you think?

I think the advantage of having InspectorDebuggerAgent::clearAsyncStackTraceData do the remove (via didCancelAsyncStackTrace) and leaving the assert in place is that we then catch any situations where nodes are erroneously left in the tree, or the tree structure is in a bad state somehow.

[Matt Baker]

After a little more thought I think this is fine. We don't have any context for why the trace is being destroyed in this case, but the tree structure is still sanity checked by the ASSERT which is seems sufficient.

[Matt Baker]

Created attachment 300049 [details]
Patch for landing

[Matt Baker]

Created attachment 300051 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 300051 [details]
Patch for landing

Clearing flags on attachment: 300051

Committed r211345: <http://trac.webkit.org/changeset/211345>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

The LayoutTest edited with this change is failing an assertion:

inspector/debugger/async-stack-trace.html
https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/r211365%20(10686)/results.html

ASSERTION FAILED: !m_childCount
/Volumes/Data/slave/elcapitan-debug/build/Source/JavaScriptCore/inspector/AsyncStackTrace.cpp(53) : Inspector::AsyncStackTrace::~AsyncStackTrace()
1   0x114093410 WTFCrash
2   0x113059724 Inspector::AsyncStackTrace::~AsyncStackTrace()
3   0x113059835 Inspector::AsyncStackTrace::~AsyncStackTrace()
4   0x11305ddd9 WTF::RefCounted<Inspector::AsyncStackTrace>::deref() const
5   0x11305dd84 void WTF::derefIfNotNull<Inspector::AsyncStackTrace>(Inspector::AsyncStackTrace*)
6   0x11305a78b WTF::RefPtr<Inspector::AsyncStackTrace>::operator=(std::nullptr_t)
7   0x113059815 Inspector::AsyncStackTrace::remove()
8   0x1130596ea Inspector::AsyncStackTrace::~AsyncStackTrace()
9   0x113059835 Inspector::AsyncStackTrace::~AsyncStackTrace()
10  0x11305ddd9 WTF::RefCounted<Inspector::AsyncStackTrace>::deref() const
11  0x11305dd84 void WTF::derefIfNotNull<Inspector::AsyncStackTrace>(Inspector::AsyncStackTrace*)
12  0x11305a78b WTF::RefPtr<Inspector::AsyncStackTrace>::operator=(std::nullptr_t)
13  0x113059815 Inspector::AsyncStackTrace::remove()
14  0x113059c2f Inspector::AsyncStackTrace::didDispatchAsyncCall()
15  0x1139b9f72 Inspector::InspectorDebuggerAgent::didDispatchAsyncCall()
16  0x1179d2fc8 WebCore::InspectorInstrumentation::didFireAnimationFrameImpl(WebCore::InspectorInstrumentationCookie const&)
17  0x118e6c9f8 WebCore::InspectorInstrumentation::didFireAnimationFrame(WebCore::InspectorInstrumentationCookie const&)
18  0x118e6c22f WebCore::ScriptedAnimationController::serviceScriptedAnimations(double)
19  0x118e6c361 WebCore::ScriptedAnimationController::displayRefreshFired()
20  0x117088a84 WebCore::DisplayRefreshMonitorClient::fireDisplayRefreshIfNeeded()
21  0x1170853f9 WebCore::DisplayRefreshMonitor::displayDidRefresh()
22  0x11708531d WebCore::DisplayRefreshMonitor::handleDisplayRefreshedNotificationOnMainThread(void*)
23  0x117089573 WebCore::DisplayRefreshMonitorMac::displayLinkFired()::$_0::operator()() const
24  0x1170894ac WTF::Function<void ()>::CallableWrapper<WebCore::DisplayRefreshMonitorMac::displayLinkFired()::$_0>::call()
25  0x1140c8283 WTF::Function<void ()>::operator()() const
26  0x1140e41cb WTF::RunLoop::performWork()
27  0x1140e5a74 WTF::RunLoop::performWork(void*)
28  0x7fff982e57e1 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
29  0x7fff982c4f1c __CFRunLoopDoSources0
30  0x7fff982c443f __CFRunLoopRun
31  0x7fff982c3e38 CFRunLoopRunSpecific

[Ryan Haddad]

Reverted r211345 for reason:

The LayoutTest for this change is failing an assertion.

Committed r211381: <http://trac.webkit.org/changeset/211381>

[Matt Baker]

Fixing.

[Matt Baker]

Created attachment 300137 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 300137 [details]
Patch for landing

Clearing flags on attachment: 300137

Committed r211385: <http://trac.webkit.org/changeset/211385>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.