177770 – SharedStringHashStore should support removing hashes

RESOLVED FIXED 177770

SharedStringHashStore should support removing hashes

https://bugs.webkit.org/show_bug.cgi?id=177770

Summary SharedStringHashStore should support removing hashes

Chris Dumez

Reported 2017-10-02 13:19:32 PDT

SharedStringHashStore should support removing hashes. It currently only supports adding hashes or clearing all of them, which is sufficient for the VisitedLinkStore but will not be for Service Worker purposes.

Attachments
WIP Patch (9.88 KB, patch) 2017-10-02 13:55 PDT, Chris Dumez	buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (3.01 MB, application/zip) 2017-10-02 15:18 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (958.56 KB, application/zip) 2017-10-02 15:31 PDT, Build Bot	no flags	Details
WIP Patch (10.06 KB, patch) 2017-10-02 15:40 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (25.02 KB, patch) 2017-10-02 16:27 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (26.42 KB, patch) 2017-10-02 16:32 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (26.53 KB, patch) 2017-10-03 08:43 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (26.56 KB, patch) 2017-10-03 11:43 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (26.66 KB, patch) 2017-10-03 14:33 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (28.12 KB, patch) 2017-10-03 14:59 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Show Obsolete (9) View All Add attachment proposed patch, testcase, etc.

Chris Dumez

Comment 1 2017-10-02 13:55:52 PDT

Created attachment 322430 [details] WIP Patch

Build Bot

Comment 2 2017-10-02 15:18:50 PDT

Comment on attachment 322430 [details] WIP Patch Attachment 322430 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4729928 New failing tests: fast/history/nested-visited-test.html fast/history/visited-link-caret-color.html fast/history/visited-inside-any.html fast/history/visited-link-background-color.html http/tests/misc/acid3.html

Build Bot

Comment 3 2017-10-02 15:18:52 PDT

Created attachment 322456 [details] Archive of layout-test-results from ews107 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6

Build Bot

Comment 4 2017-10-02 15:31:31 PDT

Comment on attachment 322430 [details] WIP Patch Attachment 322430 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4730025 Number of test failures exceeded the failure limit.

Build Bot

Comment 5 2017-10-02 15:31:33 PDT

Created attachment 322459 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6

Chris Dumez

Comment 6 2017-10-02 15:40:33 PDT

Created attachment 322462 [details] WIP Patch

Chris Dumez

Comment 7 2017-10-02 16:27:27 PDT

Created attachment 322467 [details] Patch

Chris Dumez

Comment 8 2017-10-02 16:32:14 PDT

Created attachment 322468 [details] Patch

Alex Christensen

Comment 9 2017-10-02 21:30:50 PDT

Comment on attachment 322468 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=322468&action=review > Source/WebKit/UIProcess/API/Cocoa/_WKVisitedLinkStore.h:37 > +- (BOOL)containsVisitedLinkWithURL:(NSURL *)URL; > +- (void)removeVisitedLinkWithURL:(NSURL *)URL; These need availability macros.

Chris Dumez

Comment 10 2017-10-03 08:43:36 PDT

Created attachment 322526 [details] Patch

Chris Dumez

Comment 11 2017-10-03 11:43:44 PDT

Created attachment 322553 [details] Patch

Alex Christensen

Comment 12 2017-10-03 14:24:01 PDT

Comment on attachment 322553 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=322553&action=review > Source/WebKit/Shared/SharedStringHashTable.cpp:107 > +bool SharedStringHashTable::remove(WebCore::SharedStringHash sharedStringHash) It would be nice if we could have subroutines for things like finding the slot. Also, we should add a FIXME comment indicating that sometimes we will want to rehash to save memory if the capacity becomes too low.

Chris Dumez

Comment 13 2017-10-03 14:33:12 PDT

Created attachment 322584 [details] Patch

Chris Dumez

Comment 14 2017-10-03 14:59:13 PDT

Created attachment 322586 [details] Patch

WebKit Commit Bot

Comment 15 2017-10-03 15:55:20 PDT

The commit-queue encountered the following flaky tests while processing attachment 322586 [details]: The commit-queue is continuing to process your patch.

WebKit Commit Bot

Comment 16 2017-10-03 15:55:21 PDT

The commit-queue encountered the following flaky tests while processing attachment 322586 [details]: The commit-queue is continuing to process your patch.

WebKit Commit Bot

Comment 17 2017-10-03 16:59:37 PDT

Comment on attachment 322586 [details] Patch Clearing flags on attachment: 322586 Committed r222820: <http://trac.webkit.org/changeset/222820>

WebKit Commit Bot

Comment 18 2017-10-03 16:59:39 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 19 2017-10-03 17:01:04 PDT

<rdar://problem/34802939>

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware Unspecified

OS Unspecified

Product WebKit

Component WebKit2

Assignee

Chris Dumez

Reported

2017-10-02 13:19 PDT

Modified

2017-10-09 10:21 PDT History

CC List

7 users Show

URL

Keywords InRadar

Depends on

177777

Blocks

Dependencies

tree graph