[Webkit-unassigned] [Bug 82287] MemoryCache should adopt our standard RefCounted model for object lifetime

[bugzilla-daemon at webkit.org]

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





--- Comment #18 from Nate Chapin <japhet at chromium.org>  2012-05-24 09:40:06 PST ---
(In reply to comment #17)
> (In reply to comment #16)
> > (In reply to comment #15)
> > > (From update of attachment 143386 [details] [details] [details])
> > > LGTM.  Please add some explanation to the top of CachedResource.h as well as CachedResourceHandle.h to explain when one is supposed to use a CachedResourceHandle, vs. a RefPtr.  You can do that in a separate patch, but I believe it's important to document this distinction in the code now that it's possible to confuse these two.
> > 
> > If this is a concern, then why are we doing this bizarre halfway step?
> > 
> > Understanding the MemoryCache is already difficult enough.  Making it even more confusing (but relying on comments for clarity) doesn't seem worthwhile.
> > 
> > Why can't we replace all uses of CachedResourceHandle with RefPtrs, or go 100% CachedResourceHandle with no raw pointers?
> > 
> > Wouldn't either of those be better than "confusing"?
> > 
> > (full disclosure: have not looked at the patch, have just been paying attention to the worrisome comments)
> 
> Seems worthwhile to remove cq+ for the moment.
> 
> As mentioned above, I'd be inclined to replace CachedResourceHandle with RefPtr, but I need to figure the right interactions between CachedResource and its clients in CachedResource::switchClientsToRevalidatedResource(). At the very least, I should probably include a FIXME to that effect when submitting this.

It appears to be pretty complicated to do away with CachedResourceHandle entirely (at least, not something I could throw together in a day). I'm inclined to set a rule of: RefPtrs are an implementation detail of the cache and parts of the loader below it (namely SubresourceLoader), CachedResourceHandles are the public tool for keeping CachedResources alive. Is that clear/simple enough that it might work in practice?

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