Bug 165706

Summary: WebCore::Timer is not compatible with UIProcess
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit2Assignee: Brent Fulgham <bfulgham>
Status: NEW ---    
Severity: Normal CC: aestes, andersca, bfulgham, cdumez, commit-queue
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
aestes: review+
Patch for landing.
none
Patch for landing. none

Description Brent Fulgham 2016-12-09 17:41:31 PST 
WebKit2 UIProcess code should never use WebCore::Timer as it can lead to crashes. This is especially true for applications that make use of WK2 and WK1 at the same time.

I tracked down a crash in a WebKit client to the use of WebCore::Timer in NavigationState.mm, which was introduced in https://trac.webkit.org/changeset/204716.
Comment 1 Brent Fulgham 2016-12-09 17:44:45 PST 
Created attachment 296753 [details]
Patch
Comment 2 Brent Fulgham 2016-12-09 17:45:45 PST 
<rdar://problem/29360564>
Comment 3 Andy Estes 2016-12-09 17:48:23 PST 
Comment on attachment 296753 [details]
Patch

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

> Source/WebKit2/UIProcess/Cocoa/NavigationState.h:2
> - * Copyright (C) 2014 Apple Inc. All rights reserved.
> + * Copyright (C) 2014-2016 Apple Inc. All rights reserved.

Let's also #pragma once-ify this while we're here.
Comment 4 Brent Fulgham 2016-12-09 17:50:43 PST 
Created attachment 296754 [details]
Patch for landing.
Comment 5 Brent Fulgham 2016-12-09 17:56:10 PST 
Created attachment 296755 [details]
Patch for landing.
Comment 6 Brent Fulgham 2016-12-09 18:03:21 PST 
Comment on attachment 296755 [details]
Patch for landing.

Turning of CQ+ until tests complete.
Comment 7 Brent Fulgham 2016-12-09 18:59:28 PST 
Comment on attachment 296755 [details]
Patch for landing.

Ready to land.
Comment 8 Chris Dumez 2016-12-09 19:08:49 PST 
Thank you for fixing.
Comment 9 WebKit Commit Bot 2016-12-09 19:26:11 PST 
Comment on attachment 296755 [details]
Patch for landing.

Clearing flags on attachment: 296755

Committed r209646: <http://trac.webkit.org/changeset/209646>
Comment 10 Brent Fulgham 2016-12-09 19:38:25 PST 
(In reply to comment #8)
> Thank you for fixing.

Any time! :)