[Webkit-unassigned] [Bug 51134] Move loading related code from MemoryCache to CachedResourceLoader

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Antti Koivisto <koivisto at iki.fi>  2010-12-17 01:39:55 PST ---
(In reply to comment #4)
> (From update of attachment 76761 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=76761&action=review
> 
> Looks good to me, just a couple pedantic questions/comments.
> 
> I'm going to refrain from r+ing to give other folks a chance to chime in, especially since I'm not very familiar with the MemoryCache piece of this.
> 
> > WebCore/loader/cache/CachedResourceLoader.cpp:76
> > +    default:
> > +        break;
> > +    }
> > +    return 0;
> 
> Should we have an ASSERT_NOT_REACHED() here?

Copy code but yeah, we should.

> 
> > WebCore/loader/cache/CachedResourceLoader.cpp:259
> > -    // FIXME: Consider letting the embedder block mixed content loads.
> > +
> 
> I'm not really familiar with the mixed content policy code, is this FIXME not valid anymore?

That was removed by accident (I still don't know what it means).

> 
> > WebCore/loader/cache/CachedResourceLoader.cpp:276
> > +    if (!url.isValid())
> > +        return 0;
> > +    
> > +    if (!canRequest(type, url))
> > +        return 0;
> 
> Is there any particular reason to split this out into 2 if statements?

I thought a security check deserves a line of it's own. As the comment below says, this should merge with the other similar security check but  I didn't want to change the existing behavior.

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