Bug 61407 - QuickLooks quirk is expensive to calculate
Summary: QuickLooks quirk is expensive to calculate
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-05-24 18:01 PDT by Stephanie Lewis
Modified: 2011-05-25 16:05 PDT (History)
3 users (show)

See Also:


Attachments
patch (10.82 KB, patch)
2011-05-24 19:06 PDT, Stephanie Lewis
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
patch - properly ifdef'd this time (10.97 KB, patch)
2011-05-24 19:25 PDT, Stephanie Lewis
slewis: review-
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
patch (10.97 KB, patch)
2011-05-24 19:37 PDT, Stephanie Lewis
slewis: review-
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-01 (1.21 MB, application/zip)
2011-05-24 20:05 PDT, WebKit Review Bot
no flags Details
patch (10.87 KB, patch)
2011-05-25 00:19 PDT, Stephanie Lewis
beidson: review+
beidson: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>