Bug 61407

Summary: QuickLooks quirk is expensive to calculate
Product: WebKit Reporter: Stephanie Lewis <slewis>
Component: Page LoadingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, slewis, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
webkit-ews: commit-queue-
patch - properly ifdef'd this time
slewis: review-, webkit-ews: commit-queue-
patch
slewis: review-, webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-01
none
patch beidson: review+, beidson: commit-queue-

Description Stephanie Lewis 2011-05-24 18:01:31 PDT
Remove quicklooks quirk from preferences since we don't need to set on every page.  We only care about it when the user is reloading a page.
Move the check for the quirk to after several other reload checks.
Don't use the quirk for Safari.

part of <rdar://problem/8675177>
Comment 1 Stephanie Lewis 2011-05-24 19:06:46 PDT
Created attachment 94726 [details]
patch
Comment 2 Early Warning System Bot 2011-05-24 19:16:53 PDT
Comment on attachment 94726 [details]
patch

Attachment 94726 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/8726917
Comment 3 Stephanie Lewis 2011-05-24 19:25:25 PDT
Created attachment 94728 [details]
patch - properly ifdef'd this time
Comment 4 WebKit Review Bot 2011-05-24 19:28:12 PDT
Attachment 94728 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/loader/FrameLoader.cpp:2416:  More than one command on the same line in if  [whitespace/parens] [4]
Source/WebCore/loader/FrameLoader.cpp:2419:  More than one command on the same line in if  [whitespace/parens] [4]
Total errors found: 2 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Early Warning System Bot 2011-05-24 19:35:06 PDT
Comment on attachment 94728 [details]
patch - properly ifdef'd this time

Attachment 94728 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/8734297
Comment 6 Stephanie Lewis 2011-05-24 19:37:58 PDT
Created attachment 94729 [details]
patch
Comment 7 WebKit Review Bot 2011-05-24 19:52:46 PDT
Comment on attachment 94726 [details]
patch

Attachment 94726 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8734295
Comment 8 WebKit Review Bot 2011-05-24 20:01:42 PDT
Comment on attachment 94728 [details]
patch - properly ifdef'd this time

Attachment 94728 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8734299
Comment 9 WebKit Review Bot 2011-05-24 20:05:13 PDT
Comment on attachment 94729 [details]
patch

Attachment 94729 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8726927

New failing tests:
http/tests/cache/post-redirect-get.php
Comment 10 WebKit Review Bot 2011-05-24 20:05:18 PDT
Created attachment 94732 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 11 Stephanie Lewis 2011-05-25 00:19:34 PDT
Created attachment 94748 [details]
patch
Comment 12 Brady Eidson 2011-05-25 14:22:47 PDT
Comment on attachment 94748 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=94748&action=review

r+ with the following changes:

> Source/WebCore/loader/FrameLoader.cpp:2416
> +    if (request.cachePolicy() == ReloadIgnoringCacheData && ResourceRequest::useQuickLookResourceCachingQuirks() && !equalIgnoringCase(request.httpMethod(), "post"))

You can put the !equalIgnoringCase(request.httpMethod(), "post") before the useQuickLookResourceCachingQuirks() for an even more "less likely to calculate the quicklooks quirks" setup.

> Source/WebCore/platform/network/mac/ResourceRequestMac.mm:182
> +    const char* bundleID = [[[NSBundle mainBundle] bundleIdentifier] UTF8String];
> +    if (bundleID && !strcasecmp(bundleID, "com.apple.Safari"))
> +        return false;

You should use applicationIsSafari() in RuntimeApplicationChecks
Comment 13 Stephanie Lewis 2011-05-25 16:03:43 PDT
committed http://trac.webkit.org/projects/webkit/changeset/87329
Comment 14 Stephanie Lewis 2011-05-25 16:05:48 PDT
<rdar://problem/9504417>