Bug 48979 – [Chromium] SVGListPropertyTearOff.h: function commitChange ASSERTs on Win & Mac

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Roland Steiner]

The function commitChange() in SVGListPropertyTearOff.h started to run into " ASSERT(size == values.size()); " on Chromium Windows and Mac (at least), sometimes between WK r71224 and r71305.

The failed condition seems to have no adverse effect in Release (where the ASSERT isn't run), but it still seems fishy:

'values' has 1 entry, while 'wrappers' has 2, with the first entry being NULL.

[Nikolas Zimmermann]

I'll investigate today.

[Nikolas Zimmermann]

Interessting, the garbage collection in v8 is freeing the SVGPropertyTearOff wrapper earlier than JSC, and thanks to that I found the bug:


    PassListItemTearOff removeItemValuesAndWrappers(AnimatedListPropertyTearOff* animatedList, unsigned index, ExceptionCode& ec)
    {
....
        // Detach the existing wrapper.
        RefPtr<ListItemTearOff>& oldItem = wrappers.at(index);
        if (oldItem) {
            oldItem->detachWrapper();
            wrappers.remove(index);
        }


The wrappers.remove(index) needs to be moved out of the if clause, otherwhise the list sizes don't match. Unfortunately I need to leave now :(
I think I can fix it tomorrow or tonight.
Or if anyone else wants to do that, here's how to fix :-)

[Roland Steiner]

Created attachment 73035 [details]
What Nikolas wrote

[Dirk Schulze]

Comment on attachment 73035 [details]
What Nikolas wrote

LGTM. r=me

[Nikolas Zimmermann]

Comment on attachment 73035 [details]
What Nikolas wrote

Setting cq+ to get this in ASAP, as my SVGPointList patch also hits the assertion w/o the patch and it's soon ready to be reviewed.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 73035 [details]:

http/tests/appcache/deferred-events-delete-while-raising.html

Please file bugs against the tests.  These tests were authored by [e-mail].  The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 73035 [details]
What Nikolas wrote

Clearing flags on attachment: 73035

Committed r71399: <http://trac.webkit.org/changeset/71399>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.