Bug 138750

Summary: [iOS] Crash due to null m_webPageProxyForBackForwardListForCurrentSwipe in ViewGestureController::endSwipeGesture
Product: WebKit Reporter: mitz
Component: WebKit2Assignee: mitz
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ossy, thorton
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Add some tracing
none
Ignore callbacks from dispatchAfterEnsuringDrawing for earlier swipes thorton: review+

Description mitz 2014-11-14 11:41:20 PST
<rdar://problem/18905383>

Despite the fix for bug 137770, endSwipeGesture still dereferences a null m_webPageProxyForBackForwardListForCurrentSwipe.
Comment 1 mitz 2014-11-14 11:48:03 PST
Created attachment 241609 [details]
Add some tracing
Comment 2 WebKit Commit Bot 2014-11-14 11:50:13 PST
Attachment 241609 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm:194:  Missing space around : in range-based for statement  [whitespace/colon] [4]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Tim Horton 2014-11-14 12:03:10 PST
Comment on attachment 241609 [details]
Add some tracing

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

> Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm:179
> +    WTFGetBacktrace(stack, &size);

I have long wanted a WTFStringifyBacktrace for reasons similar to this. Maybe we should put this there?
Comment 4 mitz 2014-11-14 12:05:49 PST
Committed attachment 241609 [details] as <http://trac.webkit.org/r176133>.
Comment 5 mitz 2014-11-22 04:47:19 PST
This is not fixed yet.
Comment 6 mitz 2014-12-01 13:50:43 PST
The tracing code revealed that this is probably what’s happening:

1. A first swipe gesture ends, and endSwipeGesture() schedules a call to willCommitPostSwipeTransitionLayerTree() to happen after ensuring drawing, and schedules the watchdog timer
2. Some time passes and no drawing happens, and the watchdog timer fires
3. A second swipe gesture begins
4. The second swipe gesture ends
4.1. Some drawing happens, so willCommitPostSwipeTransitionLayerTree() is finally called
4.2. setRenderTreeSize() is called, and it calls removeSwipeSnapshot()
4.3. endSwipGesture() is called and the crash happens
Comment 7 mitz 2014-12-08 14:46:25 PST
(In reply to comment #6)
> The tracing code revealed that this is probably what’s happening:
> 
> 1. A first swipe gesture ends, and endSwipeGesture() schedules a call to
> willCommitPostSwipeTransitionLayerTree() to happen after ensuring drawing,
> and schedules the watchdog timer
> 2. Some time passes and no drawing happens, and the watchdog timer fires
> 3. A second swipe gesture begins
> 4. The second swipe gesture ends
> 4.1. Some drawing happens, so willCommitPostSwipeTransitionLayerTree() is
> finally called
> 4.2. setRenderTreeSize() is called, and it calls removeSwipeSnapshot()
> 4.3. endSwipGesture() is called and the crash happens


It is actually necessary that 4.1 happen before the swipe gesture ends. I was able to reproduce this crash with a page whose loading timing I could control by making the page not load until the swipe in step 3 above started, then letting it load before the swipe ended.
Comment 8 mitz 2014-12-08 16:27:30 PST
Created attachment 242862 [details]
Ignore callbacks from dispatchAfterEnsuringDrawing for earlier swipes
Comment 9 mitz 2014-12-08 17:20:31 PST
Fixed in <http://trac.webkit.org/r176996>.