[webkit-reviews] review denied: [Bug 188174] [WinCairo] Search terms are not saved for <input type="search"> : [Attachment 352094] Save search inputs to filesystem

[bugzilla-daemon at webkit.org]

Fujii Hironori <Hironori.Fujii at sony.com> has denied Stephan Szabo
<stephan.szabo at sony.com>'s request for review:
Bug 188174: [WinCairo] Search terms are not saved for <input type="search">
https://bugs.webkit.org/show_bug.cgi?id=188174

Attachment 352094: Save search inputs to filesystem

https://bugs.webkit.org/attachment.cgi?id=352094&action=review




--- Comment #17 from Fujii Hironori <Hironori.Fujii at sony.com> ---
Comment on attachment 352094
  --> https://bugs.webkit.org/attachment.cgi?id=352094
Save search inputs to filesystem

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

> Source/WebCore/platform/win/SearchPopupMenuWin.cpp:25
> +#include <WebCore/UserAgent.h>

Do you need to include <WebCore/UserAgent.h> in this file?

> Source/WebCore/platform/win/SearchPopupMenuWin.cpp:58
> +    file.append(name);

Can `name` contain '/', '\', or '..'? How much is the maximum length? 
How do you think the idea using single SQLite database, for example 'search.db'
instead of many files?

> Source/WebCore/platform/win/SearchPopupMenuWin.cpp:72
> +void saveRecentSearches(const String& name, const
Vector<WebCore::RecentSearch>& searchItems)

I think you shouldn't use 'WebCore::' in WebCore.

> Source/WebCore/platform/win/SearchPopupMenuWin.cpp:91
> +	   auto nullTerminatedSearchItem =
WTF::stringToNullTerminatedWChar(searchItems[i].string);

Remove 'WTF::' because there is using statement.
https://github.com/WebKit/webkit/blob/2fde85661b4bf2a3ad6344694401d91e9220e99e/
Source/WTF/wtf/text/win/WCharStringExtras.h#L68