Bug 25132 - help prevent crash in chromium event handling
Summary: help prevent crash in chromium event handling
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Dmitry Titov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-10 13:19 PDT by Antony Sargent
Modified: 2009-04-14 12:00 PDT (History)
0 users

See Also:


Attachments
patch (3.27 KB, patch)
2009-04-10 13:25 PDT, Antony Sargent
dglazkov: review-
Details | Formatted Diff | Diff
patch v2 (3.29 KB, patch)
2009-04-10 13:57 PDT, Antony Sargent
dglazkov: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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