Title: [230456] trunk
Revision
230456
Author
commit-qu...@webkit.org
Date
2018-04-09 17:17:49 -0700 (Mon, 09 Apr 2018)

Log 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_...@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:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (230455 => 230456)


--- trunk/Source/WebCore/ChangeLog	2018-04-09 23:44:56 UTC (rev 230455)
+++ trunk/Source/WebCore/ChangeLog	2018-04-10 00:17:49 UTC (rev 230456)
@@ -1,3 +1,24 @@
+2018-04-09  Sihui Liu  <sihui_...@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  <bfulg...@apple.com>
 
         WebCore::EventHandler::targetPositionInWindowForSelectionAutoscroll is directly accessing NSScreen

Modified: trunk/Source/WebCore/platform/sql/SQLiteStatement.cpp (230455 => 230456)


--- trunk/Source/WebCore/platform/sql/SQLiteStatement.cpp	2018-04-09 23:44:56 UTC (rev 230455)
+++ trunk/Source/WebCore/platform/sql/SQLiteStatement.cpp	2018-04-10 00:17:49 UTC (rev 230456)
@@ -383,8 +383,6 @@
         return String();
 
     const void* blob = sqlite3_column_blob(m_statement, col);
-    if (m_database.lastError() != SQLITE_OK)
-        return String();
     if (!blob)
         return emptyString();
 

Modified: trunk/Tools/ChangeLog (230455 => 230456)


--- trunk/Tools/ChangeLog	2018-04-09 23:44:56 UTC (rev 230455)
+++ trunk/Tools/ChangeLog	2018-04-10 00:17:49 UTC (rev 230456)
@@ -1,3 +1,15 @@
+2018-04-09  Sihui Liu  <sihui_...@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  <zdober...@igalia.com>
 
         Unreviewed follow-up to r230389.

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm (230455 => 230456)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm	2018-04-09 23:44:56 UTC (rev 230455)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm	2018-04-10 00:17:49 UTC (rev 230456)
@@ -78,7 +78,7 @@
         TestWebKitAPI::Util::run(&receivedScriptMessage);
         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).
         webView = nil;

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/localstorage-empty-string-value.html (230455 => 230456)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/localstorage-empty-string-value.html	2018-04-09 23:44:56 UTC (rev 230455)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/localstorage-empty-string-value.html	2018-04-10 00:17:49 UTC (rev 230456)
@@ -7,10 +7,10 @@
   if (!localStorage.getItem(testKey)) {
     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>
-</html>
\ No newline at end of file
+</html>
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to