Bug 25132 – help prevent crash in chromium event handling

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Antony Sargent]

This is some cleanup motivated by the  crash in http://crbug.com/9775 , which happens because of calling window.open inside a window.onload handler. 

These changes are just part of the fix, along with some asserts to help prevent breakage on future changes. 

Patch forthcoming. (I will also be adding a layout test for this in a separate patch.)

[Antony Sargent]

Created attachment 29395 [details]
patch

[Dimitri Glazkov (Google)]

Comment on attachment 29395 [details]
patch

Just some styling nits.

> +This is some cleanup motivated by the  crash in http://crbug.com/9775 , which
> +happens because of calling window.open inside a window.onload handler. 
> +
> +These changes are just part of the fix, along with some asserts to help prevent
> +breakage on future changes. 

We usually indent these to the same col as "Reviewed By".

> +            // TODO(asargent) this check for hidden value being !empty is a workaround for

TODO(asargent) -> FIXME:in WebKit world.

> +            // Once the fix for that is pulled into chromium we can remove the check here.
> +            if (!object->GetHiddenValue(key).IsEmpty()) {
> +              object->DeleteHiddenValue(getKey(listener->isInline()));
> +            }

no brackets around one-liners.

[Antony Sargent]

Created attachment 29396 [details]
patch v2

new patch addressing styling issues (good catches, thx!)

[Dmitry Titov]

assignign to me for landing

[Dmitry Titov]

landed: http://trac.webkit.org/changeset/42510