[webkit-reviews] review denied: [Bug 12279] Two pass loading is needed when the network has high latency and low bandwidth : [Attachment 13537] updated patch according to review (rev 20051)

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Grace Kloba <klobag at gmail.com>'s
request for review:
Bug 12279: Two pass loading is needed when the network has high latency and low
bandwidth
http://bugs.webkit.org/show_bug.cgi?id=12279

Attachment 13537: updated patch according to review (rev 20051)
http://bugs.webkit.org/attachment.cgi?id=13537&action=edit

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
So one problem I have with this patch is that there is way too much generic
terminology employed.

The term "fast display" really should be replaced with something a bit more
specific in both the ifdef and the Document methods.

This isn't so much about "fast display."  Ideally our desktop Safari is
displaying pages fast! ;)

I'd prefer USE(LOW_BANDWIDTH_DISPLAY) and
inLowBandwidthDisplay/setLowBandwidthDisplay.

The methods addExternalRequest and removeAllExternalRequests are way too
generically named.  I'd use addLowBandwidthDisplayRequest and
removeAllLowbandwidthDisplayRequests.  I know this is a bit of a mouthful, but
I think the current names don't provide any clue that they are only used with
the low bandwidth display code.

Please ifdef the include of CachedResourceClient.h in FrameLoader.h.