[Webkit-unassigned] [Bug 150019] We should be able to clear search field recent searches based on time given

[bugzilla-daemon at webkit.org]

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

--- Comment #2 from mitz at webkit.org <mitz at webkit.org> ---
Comment on attachment 262876
  --> https://bugs.webkit.org/attachment.cgi?id=262876
Patch

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

> Source/WebCore/platform/cocoa/SearchPopupMenuCocoa.h:35
> +WEBCORE_EXPORT void removeSearchFieldRecentSearchesModifiedSince(std::chrono::system_clock::time_point modifiedSince);

This can probably just be a static function in SearchPopupMenu, and then it can have a shorter name.

> Source/WebCore/platform/cocoa/SearchPopupMenuCocoa.mm:148
> +        NSMutableDictionary *emptyRecentSearchesDictionary = [NSMutableDictionary dictionaryWithObjectsAndKeys:[NSMutableDictionary dictionary], itemsKey, nil];
> +        [emptyRecentSearchesDictionary writeToFile:searchFieldRecentSearchesPlistPath() atomically:NO];

Can we just delete this file instead of writing an empty file? Does the presence of a file signify something? Does it need to include this key-value pair? Why is everything here mutable?

> Source/WebCore/platform/cocoa/SearchPopupMenuCocoa.mm:149
> +    }

Did you mean to return early after this?

> Source/WebCore/platform/cocoa/SearchPopupMenuCocoa.mm:153
> +    if (![[itemsDictionary classForCoder] isSubclassOfClass:[NSMutableDictionary class]])

What is the significance of checking this way vs. the -isKindOfClass: checks below?

> Source/WebCore/platform/cocoa/SearchPopupMenuCocoa.mm:158
> +        if (![key isKindOfClass:[NSString class]])

I mean this…

> Source/WebCore/platform/cocoa/SearchPopupMenuCocoa.mm:162
> +        if (![[nameDictionary classForCoder] isSubclassOfClass:[NSMutableDictionary class]])

…versus this.

> Source/WebKit2/Shared/WebsiteData/WebsiteDataTypes.h:48
> +    WebsiteDataTypeSearchFieldRecentSearches = 1 << 10,
>  #if ENABLE(NETSCAPE_PLUGIN_API)
> -    WebsiteDataTypePlugInData = 1 << 10,
> +    WebsiteDataTypePlugInData = 1 << 11,
>  #endif
>  #if ENABLE(MEDIA_STREAM)
> -    WebsiteDataTypeMediaDeviceIdentifier = 1 << 11,
> +    WebsiteDataTypeMediaDeviceIdentifier = 1 << 12,
>  #endif

Why insert this in the middle?

> Source/WebKit2/UIProcess/API/Cocoa/WKWebsiteDataRecordPrivate.h:32
> +WK_EXTERN NSString * const _WKWebsiteDataTypeSearchFieldRecentSearches WK_AVAILABLE(10_11, 9_0);

This is not present in 10.11 and 9.0. You should use WK_MAC_TBA and WK_IOS_TBA instead.

> Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.cpp:40
> +#if PLATFORM(COCOA)
> +#include <WebCore/SearchPopupMenuCocoa.h>
> +#endif

Conditional imports go in a separate paragraph after the main block of imports.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20151012/cf54bdf3/attachment.html>