Continue the work started in https://bugs.webkit.org/show_bug.cgi?id=213000 and https://bugs.webkit.org/show_bug.cgi?id=213064 to replace instances of whitelist in WebCore with allowlist.
Created attachment 401620 [details] Patch
Comment on attachment 401620 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401620&action=review > Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:608 > - executeSQLCommand("CREATE TABLE IF NOT EXISTS CacheWhitelistURLs (url TEXT NOT NULL ON CONFLICT FAIL, cache INTEGER NOT NULL ON CONFLICT FAIL)"); > + executeSQLCommand("CREATE TABLE IF NOT EXISTS CacheAllowlistURLs (url TEXT NOT NULL ON CONFLICT FAIL, cache INTEGER NOT NULL ON CONFLICT FAIL)"); This seems like a no-go. > Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:623 > - " DELETE FROM CacheWhitelistURLs WHERE cache = OLD.id;" > + " DELETE FROM CacheAllowlistURLs WHERE cache = OLD.id;" This seems like a no-go. > Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:728 > + SQLiteStatement statement(m_database, "INSERT INTO CacheAllowlistURLs (url, cache) VALUES (?, ?)"); This seems like a no-go. > Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:1158 > + SQLiteStatement allowlistStatement(m_database, "SELECT url FROM CacheAllowlistURLs WHERE cache=?"); This seems like a no-go. > Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:1173 > + SQLiteStatement allowlistWildcardStatement(m_database, "SELECT wildcard FROM CacheAllowsAllNetworkRequests WHERE cache=?"); This seems like a no-go.
(But it's possible someone Databasey can say that they're OK)
Created attachment 401656 [details] Patch Here's a patch to try to make the Windows bots happy. I'll ping Brady and Sihui about the database changes.
(In reply to Beth Dakin from comment #4) > Created attachment 401656 [details] > Patch > > Here's a patch to try to make the Windows bots happy. I'll ping Brady and > Sihui about the database changes. I think Tim's right. We need to migrate the data from the old table to new table. The current patch creates a new table with new name and leave the old table behind. For the migration part, you probably can refer to something like static bool createOrMigrateRecordsTableIfNecessary(SQLiteDatabase& database) in SQLiteIDBBackingStore.cpp: if old table exists, create a temp table and copy the data to temp table, then drop the old table and rename the temp table to new name.
Created attachment 401657 [details] Patch that leaves SQL in a functioning state
Comment on attachment 401657 [details] Patch that leaves SQL in a functioning state View in context: https://bugs.webkit.org/attachment.cgi?id=401657&action=review > Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:1158 > + SQLiteStatement allowlistStatement(m_database, "SELECT url FROM CacheWhitelistURLs WHERE cache=?"); You missed one. > Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:1173 > + SQLiteStatement allowlistWildcardStatement(m_database, "SELECT wildcard FROM CacheWhiteAllNetworkRequests WHERE cache=?"); You missed one.
(In reply to Tim Horton from comment #7) > Comment on attachment 401657 [details] > Patch that leaves SQL in a functioning state > > View in context: > https://bugs.webkit.org/attachment.cgi?id=401657&action=review > > > Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:1158 > > + SQLiteStatement allowlistStatement(m_database, "SELECT url FROM CacheWhitelistURLs WHERE cache=?"); > > You missed one. > > > Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:1173 > > + SQLiteStatement allowlistWildcardStatement(m_database, "SELECT wildcard FROM CacheWhiteAllNetworkRequests WHERE cache=?"); > > You missed one. WAIT NEVERMIND
Comment on attachment 401657 [details] Patch that leaves SQL in a functioning state View in context: https://bugs.webkit.org/attachment.cgi?id=401657&action=review >> Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:1173 >> + SQLiteStatement allowlistWildcardStatement(m_database, "SELECT wildcard FROM CacheWhiteAllNetworkRequests WHERE cache=?"); > > You missed one. Oh wait, you did actually miss this one!
Created attachment 401667 [details] Patch
Created attachment 401670 [details] Patch
Found 2 new test failures: http/tests/appcache/crash-when-navigating-away-then-back.html, imported/w3c/web-platform-tests/service-workers/service-worker/claim-fetch-with-appcache.https.html
Committed r262922: <https://trac.webkit.org/changeset/262922> All reviewed patches have been landed. Closing bug and clearing flags on attachment 401670 [details].
<rdar://problem/64268181>