Bug 25132

Summary: help prevent crash in chromium event handling
Product: WebKit Reporter: Antony Sargent <asargent>
Component: DOMAssignee: Dmitry Titov <dimich>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
patch
dglazkov: review-
patch v2 dglazkov: review+

Description Antony Sargent 2009-04-10 13:19:40 PDT
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.)
Comment 1 Antony Sargent 2009-04-10 13:25:00 PDT
Created attachment 29395 [details]
patch
Comment 2 Dimitri Glazkov (Google) 2009-04-10 13:37:26 PDT
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.
Comment 3 Antony Sargent 2009-04-10 13:57:38 PDT
Created attachment 29396 [details]
patch v2

new patch addressing styling issues (good catches, thx!)
Comment 4 Dmitry Titov 2009-04-14 11:31:05 PDT
assignign to me for landing
Comment 5 Dmitry Titov 2009-04-14 12:00:34 PDT
landed: http://trac.webkit.org/changeset/42510