RESOLVED FIXED 136534
Remove ResourceResponse::m_suggestedFilename 
https://bugs.webkit.org/show_bug.cgi?id=136534
Summary Remove ResourceResponse::m_suggestedFilename
Antti Koivisto
Reported 2014-09-04 09:57:21 PDT
It is complicating things.
Attachments
patch (26.71 KB, patch)
2014-09-04 10:18 PDT, Antti Koivisto 		ap: review+
DetailsFormatted DiffDiff
for bots (27.12 KB, patch)
2014-09-04 11:48 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Antti Koivisto
Comment 1 2014-09-04 10:18:44 PDT
Created attachment 237630 [details] patch
WebKit Commit Bot
Comment 2 2014-09-04 10:19:32 PDT
Attachment 237630 [details] did not pass style-queue: ERROR: Source/WebCore/platform/network/ResourceResponseBase.h:135: The parameter name "url" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Alexey Proskuryakov
Comment 3 2014-09-04 10:46:46 PDT
Comment on attachment 237630 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=237630&action=review r=me. > Source/WebCore/platform/network/mac/ResourceResponseMac.mm:62 > + for (const auto& header : m_httpHeaderFields) I think that "auto&" is all the rage these days, not "const auto&". > Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm:812 > + auto* headers = [NSMutableDictionary dictionaryWithObjectsAndKeys: I think that we can and should use @{} here, there seems to be no reason for the dictionary to be mutable.
Pratik Solanki
Comment 4 2014-09-04 10:47:55 PDT
Comment on attachment 237630 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=237630&action=review > Source/WebCore/platform/network/mac/ResourceResponseMac.mm:172 > +String ResourceResponse::platformSuggestedFilename() const You need an implementation for this function on the CFNetwork side as well. CFURLResponseCopySuggestedFilename is the function to call.
Antti Koivisto
Comment 5 2014-09-04 11:48:09 PDT
Created attachment 237634 [details] for bots
Antti Koivisto
Comment 6 2014-09-04 13:00:43 PDT
https://trac.webkit.org/r173272
Gyuyoung Kim
Comment 7 2014-09-04 17:50:57 PDT
(In reply to comment #6) > https://trac.webkit.org/r173272 EFL and GTK ports have been broken since r173272. http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug%20%28Build%29/builds/45053
Pratik Solanki
Comment 8 2014-09-04 17:58:38 PDT
Just removing the last argument (filename) in soup/ResourceReponse.h should fix the error I think.
Pratik Solanki
Comment 9 2014-09-04 18:10:17 PDT
Checked in a speculative build fix in https://trac.webkit.org/r173300
Pratik Solanki
Comment 10 2014-09-04 18:36:57 PDT
Another one - https://trac.webkit.org/r173301
Gyuyoung Kim
Comment 11 2014-09-04 20:16:42 PDT
Finally I fixed build break on r173305 http://trac.webkit.org/changeset/173305
Simon Fraser (smfr)
Comment 12 2014-09-04 20:22:46 PDT
Two blob tests have been consistently failing after this commit: http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r173272%20(7445)/results.html
Gyuyoung Kim
Comment 13 2014-09-04 20:36:55 PDT
GTK port needed one more fix - http://trac.webkit.org/changeset/173306
Antti Koivisto
Comment 14 2014-09-05 05:20:02 PDT
Bug 136573 for the blob issue.
mitz
Comment 15 2014-09-29 16:22:55 PDT
(In reply to comment #6) > https://trac.webkit.org/r173272 This caused bug 137239.
Note You need to log in before you can comment on or make changes to this bug.