[Webkit-unassigned] [Bug 37874] Provide mechanism to cache metadata for a resource

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Tony Gentilcore <tonyg at chromium.org>  2010-04-23 08:40:47 PST ---
(In reply to comment #8)
> (In reply to comment #7)
> > (From update of attachment 54091 [details] [details])
> > WebCore/loader/CachedResource.cpp:168
> >  +  void CachedResource::setCachedMetadata(CacheableMetadata::Type type,
> > PassRefPtr<SharedBuffer> data, FrameLoader* frameLoader)
> > Why pass a FrameLoader* here?  Usually we pass around Frame*.
> 
> No reason except my unfamiliarity with the code. Done.
> 
> Because I have a couple of questions inline below, I'm holding off on uploading
> a new patch until they are all worked out.
> 
> > 
> > WebCore/loader/CachedResource.cpp:175
> >  +      if (frameLoader && frameLoader->client())
> > client() is always non-NULL.
> 
> Changed to only test frame. Let me know if loader needs to be tested as well.
> 
> > 
> > WebCore/loader/CachedResource.h:148
> >  +      void setCachedMetadata(CacheableMetadata::Type,
> > PassRefPtr<SharedBuffer>, FrameLoader*);
> > Does CachedResource have any other dependencies on Frame?  In general, we want
> > the Frame to be less involved in loading, not more.
> 
> Not really. In the case that the resource is not cached, It has a reference to
> a DocLoader (which has a reference to Frame). Passing a frame in this way does
> feel like a strange thing to do, but it was the best I could come up with. Do
> you have any alternate ideas?
> 
> > 
> > WebCore/loader/FrameLoaderClient.h:47
> >  +      struct CacheableMetadata;
> > We prefer classes to structs.
> 
> Good, me too. I took your previous comment to add a struct too literally.
> Fixed.
> 
> > 
> > WebCore/platform/network/ResourceResponseBase.h:48
> >  +      long long m_id;
> > This whole thing is very strangely layered.  In WebKit, there's no necessary
> > connection between the client and the networking layer.  This patch somehow
> > assumes some strange round-tripping between the two.
> > 
> > Why is the client involved in this process at all?  It seems like WebCore
> > should just ask the networking layer to cache these bytes.
> 
> That does sound nice. But I'm clearly missing something about the overall
> picture of the architecture. As far as I've been able to discover, routing
> through the FrameLoaderClient is the only way to get a message back to the
> embedding application. Can you point me to the interface where WebCore can send
> a notification to the networking layer with this data.

FYI -- I found a way to get rid of the ID. Just need to figure out how to
handle type now.

> 
> > 
> > WebCore/platform/network/ResourceResponseBase.h:52
> >  +          V8_SCRIPTDATA
> > Code in this location isn't allowed to know about V8.  We need a better way of
> > assigning these type numbers.  Also, are these number persisted on disk?  We
> > might need some comments explaining the rules for modifying these bits.
> 
> Yes, it does seem odd. That's why I had initially not passed a type along with
> the metadata. That leaves it in the hands of each implementer of metadata to
> determine if the data belongs to them or not. In this case, V8Proxy would just
> see if the data starts with its magic number or not. An enum, by definition,
> means that we are naming each type of metadata that can be stored -- be it for
> JSC, V8, or something else -- and that we need a registry of those types. If
> you want to keep a type, but lose the enum, we could basically extract the
> magic number out of the stream and make the type a GUID that each implementer
> chooses (relying on the size of the space for uniqueness).

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