Changeset 230456 in webkit

Timestamp:

Apr 9, 2018 5:17:49 PM (6 years ago)

Author:

commit-queue@webkit.org

Message:

REGRESSION(r229929): localStorage is broken for WebInspector
​https://bugs.webkit.org/show_bug.cgi?id=184382
<rdar://problem/39257355>

Patch by Sihui Liu <​sihui_liu@apple.com> on 2018-04-09
Reviewed by Chris Dumez.

Source/WebCore:

Removed an if condition that caused false positive cases of database error. As per
​https://www.sqlite.org/c3ref/errcode.html, return value of sqlite3_errcode() is undefined
on successful API call, so we should not use the code to check if there is an error. We
should only use it when there is an error.
After moving this condition, LocalStorage might return empty string instead of NULL on
sqlite3_column_blob() error.

Modified a test to cover this case:
TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm.

• platform/sql/SQLiteStatement.cpp:

(WebCore::SQLiteStatement::getColumnBlobAsString):

Tools:

• TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm:

(TEST):

• TestWebKitAPI/Tests/WebKitCocoa/localstorage-empty-string-value.html:

Location:

trunk

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/sql/SQLiteStatement.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/localstorage-empty-string-value.html (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-04-09  Sihui Liu  <sihui_liu@apple.com>
+
+        REGRESSION(r229929): localStorage is broken for WebInspector
+        https://bugs.webkit.org/show_bug.cgi?id=184382
+        <rdar://problem/39257355>
+
+        Reviewed by Chris Dumez.
+
+        Removed an if condition that caused false positive cases of database error. As per 
+        https://www.sqlite.org/c3ref/errcode.html, return value of sqlite3_errcode() is undefined
+        on successful API call, so we should not use the code to check if there is an error. We
+        should only use it when there is an error.
+        After moving this condition, LocalStorage might return empty string instead of NULL on
+        sqlite3_column_blob() error.
+
+        Modified a test to cover this case: 
+        TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm.
+
+        * platform/sql/SQLiteStatement.cpp:
+        (WebCore::SQLiteStatement::getColumnBlobAsString):
+
 2018-04-09  Brent Fulgham  <bfulgham@apple.com>
 

trunk/Source/WebCore/platform/sql/SQLiteStatement.cpp

@@ -384 +384 @@
 
     const void* blob = sqlite3_column_blob(m_statement, col);
-    if (m_database.lastError() != SQLITE_OK)
-        return String();
     if (!blob)
         return emptyString();

trunk/Tools/ChangeLog

@@ +1 @@
+2018-04-09  Sihui Liu  <sihui_liu@apple.com>
+
+        REGRESSION(r229929): localStorage is broken for WebInspector
+        https://bugs.webkit.org/show_bug.cgi?id=184382
+        <rdar://problem/39257355>
+
+        Reviewed by Chris Dumez.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm:
+        (TEST):
+        * TestWebKitAPI/Tests/WebKitCocoa/localstorage-empty-string-value.html:
+
 2018-04-09  Zan Dobersek  <zdobersek@igalia.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm

@@ -79 +79 @@
         receivedScriptMessage = false;
         RetainPtr<NSString> string1 = (NSString *)[lastScriptMessage body];
-        EXPECT_WK_STREQ(@"", string1.get());
+        EXPECT_WK_STREQ(@"setItem EmptyString", string1.get());
     
         // Ditch this web view (ditch the in-memory local storage).

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/localstorage-empty-string-value.html

@@ -8 +8 @@
     localStorage.setItem(key, value);
     localStorage.setItem(testKey, 'test');
+    window.webkit.messageHandlers.testHandler.postMessage("setItem " + key);
+  } else {
+    window.webkit.messageHandlers.testHandler.postMessage("" + localStorage.getItem(key));
   }
-    
-  var result = localStorage.getItem(key);
-  window.webkit.messageHandlers.testHandler.postMessage("" + result);
 </script>
 </body>