Summary: | Implement AppCache fallback entries | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||
Component: | WebCore Misc. | Assignee: | Alexey Proskuryakov <ap> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Enhancement | Keywords: | InRadar | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Alexey Proskuryakov
2008-12-30 10:23:04 PST
Created attachment 26313 [details]
proposed patch
Comment on attachment 26313 [details] proposed patch > + if (!cache) > + cache = applicationCache(); > + if (!cache) > + return false; I suggest nesting the second if statement inside the first. > + data.clear(); > + data.append(resource->data()->data(), resource->data()->size()); We recently changed clear() so that it actually shrinks the capacity of the vector. This makes this an inefficient algorithm since it involves a free and subsequent malloc. On the other hand, if the data is going to be kept around for a while, then maybe it's the right thing to do. Another possibility is to call shrink(0) instead of clear(). Or to call resize() and then memcpy() instead of append(). > + unsigned fallbackCount = m_fallbackURLs.size(); > + for (unsigned i = 0; i < fallbackCount; ++i) { We normally use size_t to iterate vectors. I'm concerned that using a vector won't scale well if the number of URLs gets large. > + void setFallbackURLs(const Vector<std::pair<KURL, KURL> >&); I think this vector type could use a typedef. FallbackURLVector maybe? > + unsigned fallbackCount = manifest.fallbackURLs.size(); > + for (unsigned i = 0; i < fallbackCount; ++i) Again, size_t. > + unsigned newestCacheID = (unsigned)statement.getColumnInt64(2); > + group->setStorageID((unsigned)statement.getColumnInt64(0)); Lets use C++ casts instead of C casts. And for integer truncation, we normally don't cast at all -- we use local variables and rely on the compiler allowing that without a warning. Not a great approach, I guess, but I really believe in keeping the casts down to a minimum and as clear as possible. Maybe we should define our own truncation_cast function template? > + unsigned fallbackCount = fallbackURLs.size(); > + for (unsigned i = 0; i < fallbackCount; ++i) { size_t again. > + namespaceURL.setRef(String()); Makes me wish for a clearRef() function. r=me Committed as <http://trac.webkit.org/changeset/39546>. |