Bug 158809

Summary:

[Cocoa] Clean up / optimize ResourceResponse::platformLazyInit(InitLevel)

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

Platform

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, cmarcelo, commit-queue

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Chris Dumez

Reported 2016-06-15 14:47:54 PDT

Clean up / optimize ResourceResponse::platformLazyInit(InitLevel).

Attachments
Patch (13.18 KB, patch) 2016-06-15 15:02 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (13.19 KB, patch) 2016-06-15 15:21 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (13.34 KB, patch) 2016-06-15 16:58 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (13.37 KB, patch) 2016-06-15 17:16 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (13.45 KB, patch) 2016-06-15 19:06 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (13.45 KB, patch) 2016-06-15 19:28 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (13.45 KB, patch) 2016-06-15 19:58 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Chris Dumez

Comment 1 2016-06-15 15:02:27 PDT

Created attachment 281388 [details] Patch

Chris Dumez

Comment 2 2016-06-15 15:21:34 PDT

Created attachment 281392 [details] Patch

Chris Dumez

Comment 3 2016-06-15 16:58:12 PDT

Created attachment 281403 [details] Patch

Chris Dumez

Comment 4 2016-06-15 17:16:32 PDT

Created attachment 281408 [details] Patch

Darin Adler

Comment 5 2016-06-15 17:53:00 PDT

Comment on attachment 281408 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281408&action=review > Source/WebCore/platform/network/HTTPParsers.h:81 > +AtomicString extractReasonPhraseFromHTTPStatusLine(const String&); I suggest changing the argument from const String& to StringView. > Source/WebCore/platform/network/cocoa/ResourceResponseCocoa.mm:171 > + CFHTTPMessageRef messageRef = CFURLResponseGetHTTPResponse([httpResponse _CFURLResponse]); I suggest using auto here. > Source/WebCore/platform/network/cocoa/ResourceResponseCocoa.mm:174 > + RetainPtr<CFDictionaryRef> headers = adoptCF(CFHTTPMessageCopyAllHeaderFields(messageRef)); I suggest using auto for the result of adoptCF. > Source/WebCore/platform/network/cocoa/ResourceResponseCocoa.mm:179 > + CFStringRef value; > + if (CFDictionaryGetValueIfPresent(headers.get(), commonHeader, (const void **)&value)) > + headersMap.set(commonHeader, value); I suggest using const void* as the type of the local variable and doing the typecast when calling set.

Chris Dumez

Comment 6 2016-06-15 18:16:05 PDT

Comment on attachment 281408 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281408&action=review >> Source/WebCore/platform/network/HTTPParsers.h:81 >> +AtomicString extractReasonPhraseFromHTTPStatusLine(const String&); > > I suggest changing the argument from const String& to StringView. I tried doing this but the issue was that the call sites have a CFStringRef (not a String). StringView does not have a constructor that takes a CFStringRef so using StringView here would force the call sites to explicitly construct a String.

Chris Dumez

Comment 7 2016-06-15 19:06:21 PDT

Created attachment 281424 [details] Patch

Chris Dumez

Comment 8 2016-06-15 19:28:14 PDT

Created attachment 281427 [details] Patch

Chris Dumez

Comment 9 2016-06-15 19:58:47 PDT

Created attachment 281430 [details] Patch

Chris Dumez

Comment 10 2016-06-15 19:59:57 PDT

Committed r202121: <http://trac.webkit.org/changeset/202121>