Bug 23036 – Implement AppCache fallback entries

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Alexey Proskuryakov]

Fallback namespaces: URLs, used as prefix match patterns, each of which is mapped to a fallback entry. Each namespace URL has the same origin as the manifest.

[Alexey Proskuryakov]

<rdar://problem/6451666>

[Alexey Proskuryakov]

Created attachment 26313 [details]
proposed patch

[Darin Adler]

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

[Alexey Proskuryakov]

Committed as <http://trac.webkit.org/changeset/39546>.