[Webkit-unassigned] [Bug 22700] ApplicationCache should have size limit

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=22700





--- Comment #25 from Alexey Proskuryakov <ap at webkit.org>  2009-07-16 12:40:38 PDT ---
(From update of attachment 32709)
> +        https://bugs.webkit.org/show_bug.cgi?id=22700
> +
> +        https://lists.webkit.org/pipermail/webkit-dev/2009-May/007560.html

We usually put bug title after its URL in ChangeLogs.

> +    , m_estimatedSizeInDatabase(0)

I don't think that Database is the right word here - do we use it elsewhere in
appcache interface? I'd prefer "OnDisk" or "InStorage".
> +    // Note that there is no need to use a RefPtr here.
> +    // The ApplicationCacheGroup instance is guaranteed
> +    // to be alive when the timer fires since invoking
> +    // the ChromeClient callback is part of its normal
> +    // update machinery and nothing can yet cause it to
> +    // get deleted.
> +    ApplicationCacheGroup* m_cacheGroup;

You could make lines longer (I'm not sure if there's an official
recommendation, but we certainly don't wrap at 80 characters). Due to its
vertical size, this comment looks more important than it is - I usually expect
that six-line comments describe architecture.

> +    // This flag is set immediately after the ChromeClient::reachedMaxAppCacheSize()
> +    // callback is invoked as a result of the storage layer failing to save a cache
> +    // due to reaching the maximum size of the application cache database file.
> +    // This flag is used by ApplicationCacheGroup::checkIfLoadIsComplete() to decide
> +    // the course of action in case of this failure (i.e. call the ChromeClient
> +    // callback or run the failure steps).
> +    // This flag is reset in ApplicationCacheGroup::checkIfLoadIsComplete().
> +    bool m_calledReachedMaxAppCacheSize;

Ditto. I'm not sure if this even needs a comment in header.

> @@ -35,6 +35,7 @@ ApplicationCacheResource::ApplicationCac
>      : SubstituteResource(url, response, data)
>      , m_type(type)
>      , m_storageID(0)
> +    , m_size(0)
>  {
>  }

Is resource size exact, or also an estimate?
> +     while (selectURLs.step() == SQLResultRow)
> +        urls->append(selectURLs.getColumnText(0));

Too much indent before "while".

> +    if (!m_database.isOpen())
> +       return false;

Too little before "return".


Anders may have more comments about the SQL part of this patch.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.