Bug 115140

Summary: Implementors of CachedResource subclasses should be forced to decide if encoded data can be replaced
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebKit2Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, gtk-ews, japhet, webkit-ews, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: All   
Attachments:
Description Flags
Patch v1
webkit-ews: commit-queue-
Patch v2 bdakin: review+

Description Brady Eidson 2013-04-24 16:25:10 PDT 
Implementors of CachedResource subclasses should be forced to decide if encoded data can be replaced

Came out of discussion in https://bugs.webkit.org/show_bug.cgi?id=115131
Comment 1 Brady Eidson 2013-04-24 16:27:55 PDT 
Created attachment 199534 [details]
Patch v1
Comment 2 Early Warning System Bot 2013-04-24 16:32:28 PDT 
Comment on attachment 199534 [details]
Patch v1

Attachment 199534 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/178193
Comment 3 Early Warning System Bot 2013-04-24 16:34:09 PDT 
Comment on attachment 199534 [details]
Patch v1

Attachment 199534 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/96958
Comment 4 Brady Eidson 2013-04-24 16:35:49 PDT 
Oh boy, somebody creates CachedResources directly...  that's bogus.  *sigh*
Comment 5 kov's GTK+ EWS bot 2013-04-24 16:36:24 PDT 
Comment on attachment 199534 [details]
Patch v1

Attachment 199534 [details] did not pass gtk-wk2-ews (gtk-wk2):
Output: http://webkit-queues.appspot.com/results/206125
Comment 6 Brady Eidson 2013-04-24 16:38:26 PDT 
Created attachment 199535 [details]
Patch v2
Comment 7 Beth Dakin 2013-04-24 17:13:06 PDT 
Comment on attachment 199535 [details]
Patch v2

This patch looks good, but I am not sure about the name mayTryReplaceEncodedData(). "try replace" just sounds awkward. mayTryToReplaceEncodedData()?
Comment 8 Brady Eidson 2013-04-24 17:15:00 PDT 
(In reply to comment #7)
> (From update of attachment 199535 [details])
> This patch looks good, but I am not sure about the name mayTryReplaceEncodedData(). "try replace" just sounds awkward. mayTryToReplaceEncodedData()?

The wording is symmetrical with the call that this method enables, which is "tryReplaceEncodedData()"

If I recall, there was a small bit of debate on that naming a month ago, but it's what we stuck with.  =/

Thanks for the review!
Comment 9 Brady Eidson 2013-04-24 17:16:38 PDT 
http://trac.webkit.org/changeset/149079