Bug 136534 - Remove ResourceResponse::m_suggestedFilename
Summary: Remove ResourceResponse::m_suggestedFilename
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 136573
Blocks:
  Show dependency tree / graph
 
Reported: 2014-09-04 09:57 PDT by Antti Koivisto
Modified: 2014-09-29 16:22 PDT (History)
7 users (show)

See Also:

Attachments
patch (26.71 KB, patch)
2014-09-04 10:18 PDT, Antti Koivisto 		ap: review+
Details | Formatted Diff | Diff
for bots (27.12 KB, patch)
2014-09-04 11:48 PDT, Antti Koivisto 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2014-09-04 09:57:21 PDT 
It is complicating things.
Comment 1 Antti Koivisto 2014-09-04 10:18:44 PDT 
Created attachment 237630 [details]
patch
Comment 2 WebKit Commit Bot 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.
Comment 3 Alexey Proskuryakov 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.
Comment 4 Pratik Solanki 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.
Comment 5 Antti Koivisto 2014-09-04 11:48:09 PDT 
Created attachment 237634 [details]
for bots
Comment 6 Antti Koivisto 2014-09-04 13:00:43 PDT 
https://trac.webkit.org/r173272
Comment 7 Gyuyoung Kim 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
Comment 8 Pratik Solanki 2014-09-04 17:58:38 PDT 
Just removing the last argument (filename) in soup/ResourceReponse.h should fix the error I think.
Comment 9 Pratik Solanki 2014-09-04 18:10:17 PDT 
Checked in a speculative build fix in https://trac.webkit.org/r173300
Comment 10 Pratik Solanki 2014-09-04 18:36:57 PDT 
Another one -  https://trac.webkit.org/r173301
Comment 11 Gyuyoung Kim 2014-09-04 20:16:42 PDT 
Finally I fixed build break on r173305

http://trac.webkit.org/changeset/173305
Comment 12 Simon Fraser (smfr) 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
Comment 13 Gyuyoung Kim 2014-09-04 20:36:55 PDT 
GTK port needed one more fix - http://trac.webkit.org/changeset/173306
Comment 14 Antti Koivisto 2014-09-05 05:20:02 PDT 
Bug 136573 for the blob issue.
Comment 15 mitz 2014-09-29 16:22:55 PDT 
(In reply to comment #6)
> https://trac.webkit.org/r173272

This caused bug 137239.