Bug 15750 - REGRESSION(r27173): Web Inspector freezes beneath Image::drawPattern()
Summary: REGRESSION(r27173): Web Inspector freezes beneath Image::drawPattern()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: mitz
URL:
Keywords: Regression
: 15752 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-10-29 09:23 PDT by Alexey Proskuryakov
Modified: 2007-10-29 20:07 PDT (History)
2 users (show)

See Also:


Attachments
Reduction (will hang) (214 bytes, text/html)
2007-10-29 10:07 PDT, mitz
no flags Details
Clamp background tile dimensions up to 1 (30.68 KB, patch)
2007-10-29 19:12 PDT, mitz
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2007-10-29 09:23:08 PDT
Steps to reproduce:
1. Open fast/xsl/xslt-mismatched-tags-in-xslt.xml.
2. Open Web Inspector.
3. Click on a stylesheet at the left.

r27173 was an Inspector fix that has apparently uncovered a pre-existing WebCore bug.
Comment 1 mitz 2007-10-29 10:07:08 PDT
Created attachment 16937 [details]
Reduction (will hang)

The tile width gets rounded down to 0...
Comment 2 mitz 2007-10-29 11:14:40 PDT
*** Bug 15752 has been marked as a duplicate of this bug. ***
Comment 3 mitz 2007-10-29 19:12:09 PDT
Created attachment 16940 [details]
Clamp background tile dimensions up to 1
Comment 4 Adam Roben (:aroben) 2007-10-29 19:25:33 PDT
Comment on attachment 16940 [details]
Clamp background tile dimensions up to 1

+    if (!patternTransform.isInvertible()) {
+        ASSERT_NOT_REACHED();
+        // Avoid a hang under CGContextDrawTiledImage on release builds.
+        return;
+    }

I think it would be slightly better to do:

ASSERT(!patternTransform.isInvertible());
if (!patternTransform.isInvertible)
    return;

This way the failed condition will appear on the console, instead of just "NOT REACHED".

Should the test be text-only?

r=me
Comment 5 mitz 2007-10-29 19:33:15 PDT
(In reply to comment #4)
> I think it would be slightly better to do:
> 
> ASSERT(!patternTransform.isInvertible());
> if (!patternTransform.isInvertible)
>     return;

Actually, that would be the wrong thing to assert :-)
I think the ASSERT_NOT_REACHED style is more readable, but I'll change to the explicit (but correct) assertion because of console output.

> Should the test be text-only?

Since it's a rendering test, I feel that the pixel result gives a little better regression protection than just "didn't crash".
Comment 6 mitz 2007-10-29 20:07:46 PDT
Landed in <http://trac.webkit.org/projects/webkit/changeset/27251>.