[WK2] Crash when accessing window.localStorage after calling window.close()

[Daniel Bates]

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

[Daniel Bates]

<rdar://problem/17315237>

[Daniel Bates]

Created attachment 235583 [details]
Patch

[Sam Weinig]

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.

[Daniel Bates]

(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.

[Daniel Bates]

Created attachment 235585 [details]
Patch

[Daniel Bates]

Committed r171661: <http://trac.webkit.org/changeset/171661>

[Daniel Bates]

Filed bug #135330 to consider allowing access/modification to window.localStorage after calling window.close().