[Webkit-unassigned] [Bug 148388] We should also store the time information for recent searches

[bugzilla-daemon at webkit.org]

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

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

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

This seems to be breaking the Windows build.

> Source/WebCore/ChangeLog:4
> +        Replace Vector<String> with Vector<std::pair<String, double>> for recent searches
> +        in order to store additional time information.

Typically the summary begins with a high-level description of the change and what motivates it. Something along the lines of “Augment <input type=search>’s recent search history with the time each entry was added, in order to allow time-based clearing of search history”.

> Source/WebKit/mac/WebCoreSupport/SearchPopupMenuMac.mm:64
> +        [items addObject:@{ searchItem.first: [[NSNumber alloc] initWithDouble:searchItem.second] }];

Just like the single-object HashMap before, what is the point of a single-object dictionary? Why not just an array?

> Source/WebKit/mac/WebCoreSupport/SearchPopupMenuMac.mm:76
> +        if ([item isKindOfClass:[NSDictionary class]] && item.allKeys > 0) {

Checking if a pointer type (item.allKeys) is greater than an integer (0) doesn’t make much sense.

This also seems like it will fail to load information saved by the current version of the code, causing it to be lost.

> Source/WebKit2/UIProcess/Cocoa/WebPageProxyCocoa.mm:54
>          items = adoptCF(CFArrayCreateMutable(0, searchItems.size(), &kCFTypeArrayCallBacks));
>  
> -        for (const auto& searchItem : searchItems)
> -            CFArrayAppendValue(items.get(), searchItem.createCFString().get());
> +        for (const auto& searchItem : searchItems) {
> +            const void* searchItemString[] = { searchItem.first };
> +            const void* searchItemDate[] = { CFNumberCreate(kCFAllocatorDefault, kCFNumberDoubleType, &searchItem.second) };
> +            RetainPtr<CFDictionaryRef> searchItemMap = adoptCF(CFDictionaryCreate(kCFAllocatorDefault, searchItemString, searchItemDate, 1, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));
> +            CFArrayAppendValue(items.get(), searchItemMap.get());

Seems weird to use CF API instead of Foundation API in this Objective-C file.

> Source/WebKit2/UIProcess/Cocoa/WebPageProxyCocoa.mm:81
> +        if (auto item = dynamic_cf_cast<CFDictionaryRef>(CFArrayGetValueAtIndex(items.get(), i))) {
> +            CFIndex size = CFDictionaryGetCount(item);
> +            Vector<CFStringRef, 1> keys(size);
> +            Vector<CFNumberRef, 1> values(size);
> +            CFDictionaryGetKeysAndValues(item, reinterpret_cast<const void**>(keys.data()), reinterpret_cast<const void**>(values.data()));
> +            if (size > 0)
> +                searchItems.append(std::make_pair((String)keys[0], ((__bridge NSNumber*)values[0]).doubleValue));
> +        }

Ditto.

-- 
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/20150825/068d6549/attachment-0001.html>