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
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

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>