Bug 180210 - Clear WebSWClientConnection in case storage process IPC connection is closing
Summary: Clear WebSWClientConnection in case storage process IPC connection is closing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-11-30 11:33 PST by youenn fablet
Modified: 2017-12-01 10:34 PST (History)
10 users (show)

See Also:

Attachments
Patch (9.93 KB, patch)
2017-11-30 11:42 PST, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch (34.88 KB, patch)
2017-11-30 11:46 PST, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch (11.09 KB, patch)
2017-11-30 14:03 PST, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch for landing (11.06 KB, patch)
2017-12-01 09:27 PST, youenn fablet 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2017-11-30 11:33:29 PST 
Clear WebSWClientConnection in case storage process IPC connection is closing
Comment 1 youenn fablet 2017-11-30 11:42:51 PST 
Created attachment 328000 [details]
Patch
Comment 2 youenn fablet 2017-11-30 11:46:54 PST 
Created attachment 328002 [details]
Patch
Comment 3 Chris Dumez 2017-11-30 11:51:33 PST 
Comment on attachment 328000 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        No chnage of behavior.

typo: chnage.

Also, this does not seem to be true as it should fix flaky crashes on the bots.

> Source/WebCore/dom/Document.h:45
> +#include "SWClientConnection.h"

Do we really have to include this here? We should try and minimize new header includes on Document.h as much as possible because of build times.

> Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp:188
> +        callback({ });

What about all the pending ServiceWorkerJobs? Shouldn't we reject their promises and clear m_scheduledJobs?
Comment 4 youenn fablet 2017-11-30 14:03:36 PST 
Created attachment 328022 [details]
Patch
Comment 5 Chris Dumez 2017-11-30 15:43:36 PST 
Comment on attachment 328022 [details]
Patch

r=me
Comment 6 WebKit Commit Bot 2017-11-30 15:45:53 PST 
Comment on attachment 328022 [details]
Patch

Rejecting attachment 328022 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 328022, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
 Skipping patch.
1 out of 1 hunk ignored
patching file Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp
patching file Source/WebKit/WebProcess/Storage/WebSWClientConnection.h
patching file Source/WebKit/WebProcess/Storage/WebToStorageProcessConnection.cpp
patching file Source/WebKit/WebProcess/Storage/WebToStorageProcessConnection.h

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Chris Dumez']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.webkit.org/results/5427171
Comment 7 youenn fablet 2017-12-01 09:27:43 PST 
Created attachment 328101 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2017-12-01 10:31:53 PST 
Comment on attachment 328101 [details]
Patch for landing

Clearing flags on attachment: 328101

Committed r225399: <https://trac.webkit.org/changeset/225399>
Comment 9 WebKit Commit Bot 2017-12-01 10:31:54 PST 
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2017-12-01 10:34:00 PST 
<rdar://problem/35799710>