[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 #24 from Zach Li <zacharyli323 at gmail.com> ---
(In reply to comment #15)
> Comment on attachment 261233 [details]
> Patch v2
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261233&action=review
> 
> I think you should use std::chrono::system_clock::time_point for times
> instead of double - then you can use std::chrono::system_clock::now() to
> create a time.
> 
> You can convert between time_points and NSDates using
> 
>     NSTimeInterval timeInterval =
> std::chrono::duration_cast<std::chrono::duration<double>>(modifiedSince.
> time_since_epoch()).count();
>     NSDate *date = [NSDate dateWithTimeIntervalSince1970:timeInterval];
> 
> Does older versions of WebKit deal with the updated user defaults format?

After talking with Jessie, older versions of WebKit do not deal with the updated user defaults format. I should think of a way to migrate the preferences.

> 
> > Source/WebCore/platform/win/SearchPopupMenuWin.cpp:96
> > +        if (CFGetTypeID(item) == CFArrayGetTypeID() && CFArrayGetCount((CFArrayRef)item) == 2) {
> 
> Can use dynamic_cf_cast here.
> 
> > Source/WebKit/mac/WebCoreSupport/SearchPopupMenuMac.mm:64
> > +        [items addObject:[NSArray arrayWithObjects:searchItem.first, [[NSNumber alloc] initWithDouble:searchItem.second], nil]];
> 
> I think you should use NSDate for the times instead.
> 
> > Source/WebKit/mac/WebCoreSupport/SearchPopupMenuMac.mm:77
> > +        if ([item isKindOfClass:[NSArray class]] && ((NSArray*)item).count == 2)
> > +            searchItems.append(std::make_pair((String)item[0], ((NSNumber*)item[1]).doubleValue));
> 
> You can use dynamic_objc_cast here.

-- 
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/20150917/c0b65509/attachment.html>