Bug 130893

Summary: Replace DEPRECATED_DEFINE_STATIC_LOCAL by static NeverDestroyed<T> in loader
Product: WebKit Reporter: Sergio Villar Senin <svillar>
Component: New BugsAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, bfulgham, buildbot, commit-queue, darin, japhet, mjs, rniwa, simon.fraser, svillar
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 130185    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion
none
Patch (with win build fix) darin: review+

Sergio Villar Senin
Reported 2014-03-28 05:14:28 PDT
Replace DEPRECATED_DEFINE_STATIC_LOCAL by static NeverDestroyed<T> in loader
Attachments
Patch (9.78 KB, patch)
2014-03-28 05:15 PDT, Sergio Villar Senin 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion (488.15 KB, application/zip)
2014-03-28 06:34 PDT, Build Bot 		no flags
Details
Patch (with win build fix) (9.78 KB, patch)
2014-03-28 09:58 PDT, Sergio Villar Senin 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Sergio Villar Senin
Comment 1 2014-03-28 05:15:42 PDT
Created attachment 228041 [details] Patch
Build Bot
Comment 2 2014-03-28 06:34:18 PDT
Comment on attachment 228041 [details] Patch Attachment 228041 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6330830213349376 New failing tests: compositing/columns/composited-rl-paginated-repaint.html
Build Bot
Comment 3 2014-03-28 06:34:21 PDT
Created attachment 228045 [details] Archive of layout-test-results from webkit-ews-02 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Brent Fulgham
Comment 4 2014-03-28 09:41:21 PDT
I just tried it on my Windows machine. If you use “friend class WTF::NeverDestroyed<blah blah>;” it should compile properly.
Sergio Villar Senin
Comment 5 2014-03-28 09:58:41 PDT
Created attachment 228058 [details] Patch (with win build fix)
Darin Adler
Comment 6 2014-03-28 22:11:37 PDT
Comment on attachment 228058 [details] Patch (with win build fix) View in context: https://bugs.webkit.org/attachment.cgi?id=228058&action=review > Source/WebCore/loader/cache/CachedRawResource.cpp:202 > static bool shouldIgnoreHeaderForCacheReuse(AtomicString headerName) I think const AtomicString& would be better for this argument. > Source/WebCore/loader/cache/CachedRawResource.cpp:215 > + static NeverDestroyed<HashSet<AtomicString>> m_headers; > + if (m_headers.get().isEmpty()) { > + m_headers.get().add("Accept"); > + m_headers.get().add("Cache-Control"); > + m_headers.get().add("Origin"); > + m_headers.get().add("Pragma"); > + m_headers.get().add("Purpose"); > + m_headers.get().add("Referer"); > + m_headers.get().add("User-Agent"); > } > - return m_headers.contains(headerName); > + return m_headers.get().contains(headerName); This is a bad idiom. For one thing, we don’t want to call isEmpty every time; this should be one-time initialization, not an empty check every time through. For another, calling add multiple times results in an unrolled loop. We should loop through an array instead. And generally speaking the setup should be in a separate function.
Sergio Villar Senin
Comment 7 2014-03-31 01:38:56 PDT
Committed r166488: <http://trac.webkit.org/changeset/166488>
Note You need to log in before you can comment on or make changes to this bug.