[webkit-reviews] review denied: [Bug 21493] Resources added to WebCore::Cache which should never be cached : [Attachment 24214] Patch v1

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied David Kilzer (ddkilzer)
<ddkilzer at webkit.org>'s request for review:
Bug 21493: Resources added to WebCore::Cache which should never be cached
https://bugs.webkit.org/show_bug.cgi?id=21493

Attachment 24214: Patch v1
https://bugs.webkit.org/attachment.cgi?id=24214&action=edit

------- Additional Comments from Sam Weinig <sam at webkit.org>
+    CacheControlDirectiveMap directives = m_response.cacheControlDirectives();

This will copy the CacheControlDirectiveMap.  Instead, passing a
CacheControlDirectiveMap to build up would be more efficient.

+    CacheControlDirectiveMap cacheControlDirectives =
m_response.cacheControlDirectives();
Same here.

+	 Vector<pair<String, String> > directives =
parseCacheHeader(pragmaHeader);
This will also cause a copy of the Vector.  There are a few more of these as
well.

+	 for (unsigned i = 0, max = directives.size(); i < max; ++i)
This is a bit awkward.	We usually put the max assignment on the line above.

+	     result.set(directives[i].first.lower(), directives[i].second);
Instead of calling lower, perhaps a CaseFoldingHash would be better for this.

+    static Vector<pair<String, String> > parseCacheHeader(const String&);
+    static Vector<String> parseCacheControlDirectiveValues(const String&);
These don't need to be declared in the header.	Marking the function as static
in the c++ will give them internal linkage.

r- for now.