Bug 135328 - [WK2] Crash when accessing window.localStorage after calling window.close()
Summary: [WK2] Crash when accessing window.localStorage after calling window.close()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-07-27 15:49 PDT by Daniel Bates
Modified: 2014-07-27 16:35 PDT (History)
5 users (show)

See Also:


Attachments
Patch (14.79 KB, patch)
2014-07-27 15:59 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (14.55 KB, patch)
2014-07-27 16:13 PDT, Daniel Bates
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2014-07-27 15:49:15 PDT
Accessing window.localStorage after calling window.close() causes a crash.

When window.close() is called we set the group of the associated page to a special sentinel "single page group" (via Page::setGroupName(String())). In WebKit2, there isn't a corresponding WebPageGroupProxy object known by the WebProcess. So, we cannot find the correct page group id to create a new local storage for a page that accessed local storage after calling window.close() (since the page group for the page is the special "single page group" page group).

Notice that in WebKit1 we support accessing local storage after calling window.close() as a side effect of our design decision to associate a local storage namespace with its file system path
Comment 1 Daniel Bates 2014-07-27 15:49:37 PDT
<rdar://problem/17315237>
Comment 2 Daniel Bates 2014-07-27 15:59:25 PDT
Created attachment 235583 [details]
Patch
Comment 3 Sam Weinig 2014-07-27 16:04:21 PDT
Comment on attachment 235583 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=235583&action=review

> Source/WebCore/page/Page.h:329
> +    bool willCloseWindowSoon() const { return m_willCloseWindowSoon && isInWindow(); }

Does this work for background tabs? I'm not clear on what the isInWindow() case is trying to achieve.

> Source/WebCore/page/Page.h:597
> +    bool m_willCloseWindowSoon;

I think a better name would be something along the lines of m_isClosing.
Comment 4 Daniel Bates 2014-07-27 16:10:01 PDT
(In reply to comment #3)
> (From update of attachment 235583 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=235583&action=review
> 
> > Source/WebCore/page/Page.h:329
> > +    bool willCloseWindowSoon() const { return m_willCloseWindowSoon && isInWindow(); }
> 
> Does this work for background tabs? I'm not clear on what the isInWindow() case is trying to achieve.

As per our in-person conversation, it's sufficient to remove the isInWindow() conjunct. I will also remove the ASSERT(isInWindow()) from the corresponding getter function.

> 
> > Source/WebCore/page/Page.h:597
> > +    bool m_willCloseWindowSoon;
> 
> I think a better name would be something along the lines of m_isClosing.

Will rename.
Comment 5 Daniel Bates 2014-07-27 16:13:51 PDT
Created attachment 235585 [details]
Patch
Comment 6 Daniel Bates 2014-07-27 16:33:46 PDT
Committed r171661: <http://trac.webkit.org/changeset/171661>
Comment 7 Daniel Bates 2014-07-27 16:35:32 PDT
Filed bug #135330 to consider allowing access/modification to window.localStorage after calling window.close().