Bug 130893 - Replace DEPRECATED_DEFINE_STATIC_LOCAL by static NeverDestroyed<T> in loader
Summary: Replace DEPRECATED_DEFINE_STATIC_LOCAL by static NeverDestroyed<T> in loader
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sergio Villar Senin
URL:
Keywords:
Depends on:
Blocks: 130185
  Show dependency treegraph
 
Reported: 2014-03-28 05:14 PDT by Sergio Villar Senin
Modified: 2014-03-31 01:38 PDT (History)
10 users (show)

See Also:


Attachments
Patch (9.78 KB, patch)
2014-03-28 05:15 PDT, Sergio Villar Senin
no flags Details | Formatted Diff | Diff
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+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sergio Villar Senin 2014-03-28 05:14:28 PDT
Replace DEPRECATED_DEFINE_STATIC_LOCAL by static NeverDestroyed<T> in loader
Comment 1 Sergio Villar Senin 2014-03-28 05:15:42 PDT
Created attachment 228041 [details]
Patch
Comment 2 Build Bot 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
Comment 3 Build Bot 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
Comment 4 Brent Fulgham 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.
Comment 5 Sergio Villar Senin 2014-03-28 09:58:41 PDT
Created attachment 228058 [details]
Patch (with win build fix)
Comment 6 Darin Adler 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.
Comment 7 Sergio Villar Senin 2014-03-31 01:38:56 PDT
Committed r166488: <http://trac.webkit.org/changeset/166488>