[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 bugzilla-daemon at webkit.org
Wed Oct 8 20:47:17 PDT 2008

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

Attachment 24214: Patch v1

------- 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 =
Same here.

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

+	 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.

More information about the webkit-reviews mailing list