The following lines added in <https://webkit.org/b/69138> appear to have broken WebInspector's usage of `window.localStorage`: String SQLiteStatement::getColumnBlobAsString(int col) ...| 386| if (m_database.lastError() != SQLITE_OK) 387| return String(); ...| STEPS TO REPRODUCE: 1. Open WebInspector 2. Do anything that would be "saved" for the next load (e.g. move around tabs, change settings, modify sidebar sizes, toggle paint flashing, etc.) 3. Close WebInspector 4. Reopen WebInspector => Any changes previously made will no longer be there (e.g. tabs will be back in original order, settings will revert to default value, all sidebars will be back to default size, paint flashing is disabled, etc.) NOTES: The value of `m_database.lastErrorMessage()` seems to be "unknown error" throughout `getColumnBlobAsString` (this was determined by using `syslog(LOG_ERR, "%s", m_database.lastErrorMsg());`). In the case that `m_database.lastError() != SQLITE_OK`, `blob` isn't falsy and `getColumnBlobAsString` doesn't early return.
<rdar://problem/39257355>
Created attachment 337439 [details] Patch
Comment on attachment 337439 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337439&action=review > Source/WebCore/ChangeLog:15 > + No new tests, removed an insufficient judgement to fix regression. Is it not possible to write a test for this?
Created attachment 337453 [details] Patch
When is the error ever cleared? If there was an error before entering this method would it have been cleared?
(In reply to Joseph Pecoraro from comment #5) > When is the error ever cleared? If there was an error before entering this > method would it have been cleared? From the doc [1]: """ If the most recent sqlite3_* API call associated with database connection D failed, then the sqlite3_errcode(D) interface returns the numeric result code or extended result code for that API call. If the most recent API call was successful, then the return value from sqlite3_errcode() is undefined. """ So my understanding is: - If the call to sqlite3_column_blob() failed, sqlite3_errcode() will return a valid error code. - If the call to sqlite3_column_blob() succeeded, sqlite3_errcode() may return whatever. Unfortunately, it does not look like there is a good way to check if the call to sqlite3_column_blob() failed. This is because as per [2] sqlite3_column_blob() can return NULL due to: - An error - The blob has zero length [1] https://www.sqlite.org/c3ref/errcode.html [2] https://www.sqlite.org/c3ref/column_blob.html
Comment on attachment 337453 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337453&action=review > Source/WebCore/platform/sql/SQLiteStatement.cpp:388 > + if (!(error == SQLITE_OK || error == SQLITE_ROW || error == SQLITE_DONE)) As per the doc, "If the most recent API call was successful, then the return value from sqlite3_errcode() is undefined." Therefore, I do not think this patch is correct.
Created attachment 337507 [details] Patch
Created attachment 337510 [details] Patch
Comment on attachment 337510 [details] Patch Attachment 337510 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/7255578 New failing tests: imported/w3c/web-platform-tests/workers/name-property.html
Created attachment 337518 [details] Archive of layout-test-results from ews101 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 337510 [details] Patch failure is unrelated.
Comment on attachment 337510 [details] Patch Clearing flags on attachment: 337510 Committed r230456: <https://trac.webkit.org/changeset/230456>
All reviewed patches have been landed. Closing bug.