Title: [171779] tags/Safari-600.1.3.3
Revision
171779
Author
[email protected]
Date
2014-07-29 16:23:13 -0700 (Tue, 29 Jul 2014)

Log Message

Merged r171661. <rdar://problem/17315237>

Modified Paths

Added Paths

Diff

Modified: tags/Safari-600.1.3.3/LayoutTests/ChangeLog (171778 => 171779)


--- tags/Safari-600.1.3.3/LayoutTests/ChangeLog	2014-07-29 23:16:18 UTC (rev 171778)
+++ tags/Safari-600.1.3.3/LayoutTests/ChangeLog	2014-07-29 23:23:13 UTC (rev 171779)
@@ -1,3 +1,31 @@
+2014-07-29  Babak Shafiei  <[email protected]>
+
+        Merge r171661.
+
+    2014-07-27  Daniel Bates  <[email protected]>
+
+            [WK2] Crash when accessing window.localStorage after calling window.close()
+            https://bugs.webkit.org/show_bug.cgi?id=135328
+            <rdar://problem/17315237>
+
+            Reviewed by Sam Weinig.
+
+            Added test by Andy Estes, LayoutTests/storage/domstorage/localstorage/access-storage-after-window-close.html,
+            to ensure that we don't crash when accessing local storage for the first time after calling window.close().
+
+            Additionally added tests that ensure that updates to local storage are ignored after calling
+            window.close() regardless of whether local storage was accessed before the call to window.close().
+
+            * storage/domstorage/localstorage/access-storage-after-window-close-expected.txt: Added.
+            * storage/domstorage/localstorage/access-storage-after-window-close.html: Added.
+            * storage/domstorage/localstorage/access-storage-then-set-value-in-storage-after-window-close-expected.txt: Added.
+            * storage/domstorage/localstorage/access-storage-then-set-value-in-storage-after-window-close.html: Added.
+            * storage/domstorage/localstorage/resources/access-storage-close-window-and-set-value-in-storage.html: Added.
+            * storage/domstorage/localstorage/resources/close-window-and-access-storage.html: Added.
+            * storage/domstorage/localstorage/resources/close-window-and-set-value-in-storage.html: Added.
+            * storage/domstorage/localstorage/set-value-in-storage-after-window-close-expected.txt: Added.
+            * storage/domstorage/localstorage/set-value-in-storage-after-window-close.html: Added.
+
 2014-07-25  Lucas Forschler  <[email protected]>
 
         Merge r171593

Copied: tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/access-storage-after-window-close-expected.txt (from rev 171661, trunk/LayoutTests/storage/domstorage/localstorage/access-storage-after-window-close-expected.txt) (0 => 171779)


--- tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/access-storage-after-window-close-expected.txt	                        (rev 0)
+++ tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/access-storage-after-window-close-expected.txt	2014-07-29 23:23:13 UTC (rev 171779)
@@ -0,0 +1,2 @@
+Test that WebKit does not crash when accessing localStorage after calling window.close().
+

Copied: tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/access-storage-after-window-close.html (from rev 171661, trunk/LayoutTests/storage/domstorage/localstorage/access-storage-after-window-close.html) (0 => 171779)


--- tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/access-storage-after-window-close.html	                        (rev 0)
+++ tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/access-storage-after-window-close.html	2014-07-29 23:23:13 UTC (rev 171779)
@@ -0,0 +1,14 @@
+<html>
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.setCanOpenWindows();
+    testRunner.waitUntilDone();
+}
+</script>
+</head>
+<body _onload_="window.open('resources/close-window-and-access-storage.html');">
+Test that WebKit does not crash when accessing localStorage after calling window.close().<br>
+</body>
+</html>

Copied: tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/access-storage-then-set-value-in-storage-after-window-close-expected.txt (from rev 171661, trunk/LayoutTests/storage/domstorage/localstorage/access-storage-then-set-value-in-storage-after-window-close-expected.txt) (0 => 171779)


--- tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/access-storage-then-set-value-in-storage-after-window-close-expected.txt	                        (rev 0)
+++ tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/access-storage-then-set-value-in-storage-after-window-close-expected.txt	2014-07-29 23:23:13 UTC (rev 171779)
@@ -0,0 +1,11 @@
+Tests that we can't store a value in local storage after calling window.close() even if we created the local storage before the call to window.close().
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS window.localStorage is defined.
+PASS window.localStorage["test"] is undefined.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Copied: tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/access-storage-then-set-value-in-storage-after-window-close.html (from rev 171661, trunk/LayoutTests/storage/domstorage/localstorage/access-storage-then-set-value-in-storage-after-window-close.html) (0 => 171779)


--- tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/access-storage-then-set-value-in-storage-after-window-close.html	                        (rev 0)
+++ tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/access-storage-then-set-value-in-storage-after-window-close.html	2014-07-29 23:23:13 UTC (rev 171779)
@@ -0,0 +1,31 @@
+<html>
+<head>
+<script src=""
+<script>
+window.jsTestIsAsync = true;
+
+if (window.testRunner) {
+    testRunner.setCanOpenWindows();
+
+    // We explicitly call testRunner.waitUntilDone() because we call window.open() before
+    // js-test-post.js is processed.
+    testRunner.waitUntilDone();
+}
+
+function checkResultAndNotifyDone()
+{
+    shouldBeDefined('window.localStorage');
+    shouldBeUndefined('window.localStorage["test"]');
+    window.localStorage.clear();
+    finishJSTest();
+}
+</script>
+</head>
+<body>
+<script>
+description("Tests that we can't store a value in local storage after calling window.close() even if we created the local storage before the call to window.close().");
+window.open("resources/access-storage-close-window-and-set-value-in-storage.html");
+</script>
+<script src=""
+</body>
+</html>

Copied: tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/resources/access-storage-close-window-and-set-value-in-storage.html (from rev 171661, trunk/LayoutTests/storage/domstorage/localstorage/resources/access-storage-close-window-and-set-value-in-storage.html) (0 => 171779)


--- tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/resources/access-storage-close-window-and-set-value-in-storage.html	                        (rev 0)
+++ tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/resources/access-storage-close-window-and-set-value-in-storage.html	2014-07-29 23:23:13 UTC (rev 171779)
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+window.localStorage; // Creates local storage
+window.close();
+if (window.localStorage)
+    localStorage["test"] = "DidSetValueAfterCallingWindowClose";
+window.opener.checkResultAndNotifyDone();
+</script>
+</head>
+</html>

Copied: tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/resources/close-window-and-access-storage.html (from rev 171661, trunk/LayoutTests/storage/domstorage/localstorage/resources/close-window-and-access-storage.html) (0 => 171779)


--- tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/resources/close-window-and-access-storage.html	                        (rev 0)
+++ tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/resources/close-window-and-access-storage.html	2014-07-29 23:23:13 UTC (rev 171779)
@@ -0,0 +1,10 @@
+<html>
+<head>
+<script>
+window.close();
+window.localStorage;
+if (window.testRunner)
+    testRunner.notifyDone();
+</script>
+</head>
+</html>

Copied: tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/resources/close-window-and-set-value-in-storage.html (from rev 171661, trunk/LayoutTests/storage/domstorage/localstorage/resources/close-window-and-set-value-in-storage.html) (0 => 171779)


--- tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/resources/close-window-and-set-value-in-storage.html	                        (rev 0)
+++ tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/resources/close-window-and-set-value-in-storage.html	2014-07-29 23:23:13 UTC (rev 171779)
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+window.close();
+if (window.localStorage)
+    localStorage["test"] = "DidSetValueAfterCallingWindowClose";
+window.opener.checkResultAndNotifyDone();
+</script>
+</head>
+</html>

Copied: tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/set-value-in-storage-after-window-close-expected.txt (from rev 171661, trunk/LayoutTests/storage/domstorage/localstorage/set-value-in-storage-after-window-close-expected.txt) (0 => 171779)


--- tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/set-value-in-storage-after-window-close-expected.txt	                        (rev 0)
+++ tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/set-value-in-storage-after-window-close-expected.txt	2014-07-29 23:23:13 UTC (rev 171779)
@@ -0,0 +1,11 @@
+Tests that we can't store a value in local storage after calling window.close().
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS window.localStorage is defined.
+PASS window.localStorage["test"] is undefined.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Copied: tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/set-value-in-storage-after-window-close.html (from rev 171661, trunk/LayoutTests/storage/domstorage/localstorage/set-value-in-storage-after-window-close.html) (0 => 171779)


--- tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/set-value-in-storage-after-window-close.html	                        (rev 0)
+++ tags/Safari-600.1.3.3/LayoutTests/storage/domstorage/localstorage/set-value-in-storage-after-window-close.html	2014-07-29 23:23:13 UTC (rev 171779)
@@ -0,0 +1,31 @@
+<html>
+<head>
+<script src=""
+<script>
+window.jsTestIsAsync = true;
+
+if (window.testRunner) {
+    testRunner.setCanOpenWindows();
+
+    // We explicitly call testRunner.waitUntilDone() because we call window.open() before
+    // js-test-post.js is processed.
+    testRunner.waitUntilDone();
+}
+
+function checkResultAndNotifyDone()
+{
+    shouldBeDefined('window.localStorage');
+    shouldBeUndefined('window.localStorage["test"]');
+    window.localStorage.clear();
+    finishJSTest();
+}
+</script>
+</head>
+<body>
+<script>
+description("Tests that we can't store a value in local storage after calling window.close().");
+window.open("resources/close-window-and-set-value-in-storage.html");
+</script>
+<script src=""
+</body>
+</html>

Modified: tags/Safari-600.1.3.3/Source/WebCore/ChangeLog (171778 => 171779)


--- tags/Safari-600.1.3.3/Source/WebCore/ChangeLog	2014-07-29 23:16:18 UTC (rev 171778)
+++ tags/Safari-600.1.3.3/Source/WebCore/ChangeLog	2014-07-29 23:23:13 UTC (rev 171779)
@@ -1,5 +1,40 @@
 2014-07-29  Babak Shafiei  <[email protected]>
 
+        Merge r171661.
+
+    2014-07-27  Daniel Bates  <[email protected]>
+
+            [WK2] Crash when accessing window.localStorage after calling window.close()
+            https://bugs.webkit.org/show_bug.cgi?id=135328
+            <rdar://problem/17315237>
+
+            Reviewed by Sam Weinig.
+
+            Fixes an issue where accessing local storage for the first time after calling window.close()
+            causes a crash.
+
+            For now, we should disallow accessing local storage after calling window.close() regardless of
+            whether it's the first access to local storage as this seems like a bad idiom to support. Note,
+            this represents a change in behavior from WebKit1. If such usage of window.localStorage turns
+            out to be reasonable then we can visit this decision again in <https://bugs.webkit.org/show_bug.cgi?id=135330>.
+
+            Tests: storage/domstorage/localstorage/access-storage-after-window-close.html
+                   storage/domstorage/localstorage/access-storage-then-set-value-in-storage-after-window-close.html
+                   storage/domstorage/localstorage/set-value-in-storage-after-window-close.html
+
+            * page/DOMWindow.cpp:
+            (WebCore::DOMWindow::localStorage): Modified to only return the cached local storage or
+            create a new local storage so long as the page isn't being closed. Also, substitute nullptr
+            for 0.
+            (WebCore::DOMWindow::close): Call Page::setIsClosing() to mark that the page is closing.
+            * page/Page.cpp:
+            (WebCore::Page::Page): Initialize m_isClosing to false.
+            * page/Page.h:
+            (WebCore::Page::setIsClosing): Added.
+            (WebCore::Page::isClosing): Added.
+
+2014-07-29  Babak Shafiei  <[email protected]>
+
         Merge r171647.
 
     2014-07-26  Timothy Horton  <[email protected]>

Modified: tags/Safari-600.1.3.3/Source/WebCore/page/DOMWindow.cpp (171778 => 171779)


--- tags/Safari-600.1.3.3/Source/WebCore/page/DOMWindow.cpp	2014-07-29 23:16:18 UTC (rev 171778)
+++ tags/Safari-600.1.3.3/Source/WebCore/page/DOMWindow.cpp	2014-07-29 23:23:13 UTC (rev 171779)
@@ -821,31 +821,38 @@
 Storage* DOMWindow::localStorage(ExceptionCode& ec) const
 {
     if (!isCurrentlyDisplayedInFrame())
-        return 0;
+        return nullptr;
 
     Document* document = this->document();
     if (!document)
-        return 0;
+        return nullptr;
 
     if (!document->securityOrigin()->canAccessLocalStorage(0)) {
         ec = SECURITY_ERR;
-        return 0;
+        return nullptr;
     }
 
-    if (m_localStorage) {
-        if (!m_localStorage->area().canAccessStorage(m_frame)) {
-            ec = SECURITY_ERR;
-            return 0;
+    Page* page = document->page();
+    // FIXME: We should consider supporting access/modification to local storage
+    // after calling window.close(). See <https://bugs.webkit.org/show_bug.cgi?id=135330>.
+    if (!page || !page->isClosing()) {
+        if (m_localStorage) {
+            if (!m_localStorage->area().canAccessStorage(m_frame)) {
+                ec = SECURITY_ERR;
+                return nullptr;
+            }
+            return m_localStorage.get();
         }
-        return m_localStorage.get();
     }
 
-    Page* page = document->page();
     if (!page)
-        return 0;
+        return nullptr;
 
+    if (page->isClosing())
+        return nullptr;
+
     if (!page->settings().localStorageEnabled())
-        return 0;
+        return nullptr;
 
     RefPtr<StorageArea> storageArea;
     if (!document->securityOrigin()->canAccessLocalStorage(document->topOrigin()))
@@ -855,7 +862,7 @@
 
     if (!storageArea->canAccessStorage(m_frame)) {
         ec = SECURITY_ERR;
-        return 0;
+        return nullptr;
     }
 
     m_localStorage = Storage::create(m_frame, storageArea.release());
@@ -1035,6 +1042,7 @@
     if (!m_frame->loader().shouldClose())
         return;
 
+    page->setIsClosing();
     page->chrome().closeWindowSoon();
 }
 

Modified: tags/Safari-600.1.3.3/Source/WebCore/page/Page.cpp (171778 => 171779)


--- tags/Safari-600.1.3.3/Source/WebCore/page/Page.cpp	2014-07-29 23:16:18 UTC (rev 171778)
+++ tags/Safari-600.1.3.3/Source/WebCore/page/Page.cpp	2014-07-29 23:23:13 UTC (rev 171779)
@@ -201,6 +201,7 @@
     , m_userContentController(WTF::move(pageClients.userContentController))
     , m_visitedLinkStore(WTF::move(pageClients.visitedLinkStore))
     , m_sessionID(SessionID::defaultSessionID())
+    , m_isClosing(false)
 {
     ASSERT(m_editorClient);
     

Modified: tags/Safari-600.1.3.3/Source/WebCore/page/Page.h (171778 => 171779)


--- tags/Safari-600.1.3.3/Source/WebCore/page/Page.h	2014-07-29 23:16:18 UTC (rev 171778)
+++ tags/Safari-600.1.3.3/Source/WebCore/page/Page.h	2014-07-29 23:23:13 UTC (rev 171779)
@@ -320,6 +320,9 @@
     void setIsInWindow(bool);
     bool isInWindow() const { return m_viewState & ViewState::IsInWindow; }
 
+    void setIsClosing() { m_isClosing = true; }
+    bool isClosing() const { return m_isClosing; }
+
     void addViewStateChangeObserver(ViewStateChangeObserver&);
     void removeViewStateChangeObserver(ViewStateChangeObserver&);
 
@@ -585,6 +588,8 @@
     HashSet<ViewStateChangeObserver*> m_viewStateChangeObservers;
 
     SessionID m_sessionID;
+
+    bool m_isClosing;
 };
 
 inline PageGroup& Page::group()
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to