Bug 121232 - If UI delegate abort window.close() then second window.close() is a no-op
Summary: If UI delegate abort window.close() then second window.close() is a no-op
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-12 10:06 PDT by Allan Odgaard
Modified: 2016-08-11 02:19 PDT (History)
3 users (show)

See Also:


Attachments
Fix missing zeroing of timer (340 bytes, patch)
2013-10-02 03:18 PDT, Allan Odgaard
no flags Details | Formatted Diff | Diff
Fix missing zeroing of timer (2) (372 bytes, patch)
2013-10-02 03:30 PDT, Allan Odgaard
no flags Details | Formatted Diff | Diff

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