Bug 121232

Summary: If UI delegate abort window.close() then second window.close() is a no-op
Product: WebKit Reporter: Allan Odgaard <CEC49BC5-C186-483E-A513-9DF2DEEAE27E>
Component: WebKit APIAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: ap, bfulgham, pvollan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
Description Flags
Fix missing zeroing of timer
Fix missing zeroing of timer (2) none

Description Allan Odgaard 2013-09-12 10:06:14 PDT
In WebView::closeWindowSoon() (WebView.cpp) m_closeWindowTimer is set to a timer, unless it’s already non-zero, in which case the function immediately returns.

Since m_closeWindowTimer is never set back to zero (even after the timer has fired), it means that if the HTML calls window.close() and the UI delegate decides not to dispose the web view, then any future call to window.close() will be a no-op.

A fix is likely to let WebView::closeWindowTimerFired() set “m_closeWindowTimer = 0;” (WebView.cpp)
Comment 1 Alexey Proskuryakov 2013-09-24 10:03:06 PDT
Good catch.

Allan, would you be willing to submit a patch with a fix? Please see <http://www.webkit.org/coding/contributing.html> for some pointers and guidelines.

Changing platform from Mac to Windows, as the quoted code is in Windows WebKit.
Comment 2 Allan Odgaard 2013-10-02 03:18:16 PDT
Created attachment 213159 [details]
Fix missing zeroing of timer
Comment 3 Allan Odgaard 2013-10-02 03:20:18 PDT
It should be a one line change which I attached a patch for.

I didn’t do any changelog, run tests, etc. as I do not have a build environment for WebKit. I ran into the issue working with the system’s standard WebView on OS X 10.8.4 and read through the source of WebKit to figure out why it didn’t behave as expected.
Comment 4 Allan Odgaard 2013-10-02 03:24:08 PDT
Disregard that patch, it leaks the timer.

Will update.
Comment 5 Allan Odgaard 2013-10-02 03:30:28 PDT
Created attachment 213161 [details]
Fix missing zeroing of timer (2)

This deletes the timer and replaces the previous patch.
Comment 6 Allan Odgaard 2016-08-10 13:17:55 PDT
Comment 7 Alexey Proskuryakov 2016-08-10 14:00:43 PDT
The patch is not marked for review, and doesn't have a ChangeLog.

While we are grateful for a contribution in any form, following the normal process can make it move faster.
Comment 8 Allan Odgaard 2016-08-11 02:19:31 PDT
As I am unfamiliar with your normal process and does not have a WebKit build environment (the patch was submitted only because it was requested from me), I would be grateful if someone else can follow the required steps to get this bug fixed.