Bug 177770 - SharedStringHashStore should support removing hashes
Summary: SharedStringHashStore should support removing hashes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on: 177777
Blocks:
  Show dependency tree / graph
 
Reported: 2017-10-02 13:19 PDT by Chris Dumez
Modified: 2017-10-09 10:21 PDT (History)
7 users (show)

See Also:

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 an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 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.
Comment 1 Chris Dumez 2017-10-02 13:55:52 PDT 
Created attachment 322430 [details]
WIP Patch
Comment 2 Build Bot 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
Comment 3 Build Bot 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
Comment 4 Build Bot 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.
Comment 5 Build Bot 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
Comment 6 Chris Dumez 2017-10-02 15:40:33 PDT 
Created attachment 322462 [details]
WIP Patch
Comment 7 Chris Dumez 2017-10-02 16:27:27 PDT 
Created attachment 322467 [details]
Patch
Comment 8 Chris Dumez 2017-10-02 16:32:14 PDT 
Created attachment 322468 [details]
Patch
Comment 9 Alex Christensen 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.
Comment 10 Chris Dumez 2017-10-03 08:43:36 PDT 
Created attachment 322526 [details]
Patch
Comment 11 Chris Dumez 2017-10-03 11:43:44 PDT 
Created attachment 322553 [details]
Patch
Comment 12 Alex Christensen 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.
Comment 13 Chris Dumez 2017-10-03 14:33:12 PDT 
Created attachment 322584 [details]
Patch
Comment 14 Chris Dumez 2017-10-03 14:59:13 PDT 
Created attachment 322586 [details]
Patch
Comment 15 WebKit Commit Bot 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.
Comment 16 WebKit Commit Bot 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.
Comment 17 WebKit Commit Bot 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>
Comment 18 WebKit Commit Bot 2017-10-03 16:59:39 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 19 Radar WebKit Bug Importer 2017-10-03 17:01:04 PDT 
<rdar://problem/34802939>