[webkit-reviews] review denied: [Bug 30209] Cache::m_liveDecodedResource is not sorted by CachedResource::m_lastDecodedAccessTime : [Attachment 41860] Comment the behavior of the live resources list

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Holger Freyther <zecke at selfish.org>'s
request for review:
Bug 30209: Cache::m_liveDecodedResource is not sorted by
CachedResource::m_lastDecodedAccessTime
https://bugs.webkit.org/show_bug.cgi?id=30209

Attachment 41860: Comment the behavior of the live resources list
https://bugs.webkit.org/attachment.cgi?id=41860&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
Looks good.

> +    // The list is not stricly sorted by the m_lastDecodedAccessTime but

Typo here "stricly".

> +    // the impact of this behavior is minor as the below if with the return
> +    // statement will not evaluate to true as the currentTime is >> than
> +    // current->m_lastDecodedAccessTime. For more details see:
> +    // https://bugs.webkit.org/show_bug.cgi?id=30209

I am not sure that the term "not strictly sorted" is the right one here.
Typically "strict" sorting simply means a sort where no two items are
considered equal.

Maybe you mean "does not maintain a strict invariant" or something like that?

> +	   // Insert into or remove from the live decoded list if necessary.
When inserting
> +	   // into the LiveDecodedResourcesList the m_lastDecodedAccessTime
might still be
> +	   // zero or smaller than the m_lastDecodedAccessTime of the current
head of this
> +	   // list. This is a violation of the current invariant but it is not
a problem.

The term "the current invariant" here is confusing. I don't know what could
make something both "current" and "invariant". Different wording would be
clearer. You could just remove "current", I guess. I think it would be best to
say "a violation of the invariant that the list be kept sorted by access time".


Since the patch is entirely about comments, I'll say review- because of the
typo.