Bug 57959

Summary:

Should assert on invalid type in FEComponentTransfer::apply

Product:

WebKit

Reporter:

Justin Schuh <jschuh>

Component:

New Bugs

Assignee:

Justin Schuh <jschuh>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, eric, krit, zimmermann

Priority:

Version:

528+ (Nightly build)

Hardware:

Other

OS:

OS X 10.5

Attachments:

Description	Flags
Patch	eric: review+, commit-queue: commit-queue-

Justin Schuh

Reported 2011-04-06 10:06:47 PDT

Should assert on invalid type in FEComponentTransfer::apply

Attachments
Patch (1.79 KB, patch) 2011-04-06 10:11 PDT, Justin Schuh	eric: review+ commit-queue: commit-queue-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Justin Schuh

Comment 1 2011-04-06 10:11:36 PDT

Created attachment 88459 [details] Patch

Eric Seidel (no email)

Comment 2 2011-04-06 10:54:43 PDT

Comment on attachment 88459 [details] Patch Makes sense.

Eric Seidel (no email)

Comment 3 2011-04-06 10:55:47 PDT

Comment on attachment 88459 [details] Patch Actually, shouldn't we do this before we loop over all the rgba values (in a separate loop?) This seems it would make debug behavior needlessly slow.

Justin Schuh

Comment 4 2011-04-06 11:02:06 PDT

(In reply to comment #3) > (From update of attachment 88459 [details]) > Actually, shouldn't we do this before we loop over all the rgba values (in a separate loop?) This seems it would make debug behavior needlessly slow. I think this way is easier to follow than a duplicate loop blocked out with debug #ifdefs. And since it's an assert (that doesn't affect release), I think readability should be the priority.

Justin Schuh

Comment 5 2011-04-21 15:54:15 PDT

Eric, did I sufficiently respond to your question and do you have any objection to me landing it?

Eric Seidel (no email)

Comment 6 2011-04-22 09:18:12 PDT

I'm not standing in the way of this patch, no, you're welcome to land.

Justin Schuh

Comment 7 2011-04-22 10:08:50 PDT

(In reply to comment #6) > I'm not standing in the way of this patch, no, you're welcome to land. Thanks. It slipped off my radar and I was trying to do some cleanup.

WebKit Commit Bot

Comment 8 2011-04-22 11:48:29 PDT

The commit-queue encountered the following flaky tests while processing attachment 88459 [details]: http/tests/misc/favicon-loads-with-icon-loading-override.html bug 58412 (author: alice.liu@apple.com) The commit-queue is continuing to process your patch.

WebKit Commit Bot

Comment 9 2011-04-22 12:28:07 PDT

Comment on attachment 88459 [details] Patch Rejecting attachment 88459 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'apply-..." exit_code: 2 Last 500 characters of output: " exit_code: 1 Parsed 2 diffs from patch file(s). patching file Source/WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebCore/platform/graphics/filters/FEComponentTransfer.cpp Hunk #1 FAILED at 33. Hunk #2 FAILED at 170. 2 out of 2 hunks FAILED -- saving rejects to file Source/WebCore/platform/graphics/filters/FEComponentTransfer.cpp.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Eric Seidel', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/8486842

Justin Schuh

Comment 10 2011-04-22 15:42:25 PDT

Fixed in http://trac.webkit.org/changeset/84653

Note You need to log in before you can comment on or make changes to this bug.