Make WKOriginDataManager correctly operate on IndexedDatabases This patch will get the list of origins with databases by checking for their entries in the directory structure on disk. It will also delete the database files (and any resulting empty directories) when requested. <rdar://problem/15917314>
In this patch, open handles to the database files on disk will not be notified and invalidated. This is probably quite an acceptable situation for now. because: - For read-only operations, they will continue functioning as normal on the unlinked file. - For write operations, they will start producing errors as SQLite notices the missing backing store. But it should still be fixed, and I filed https://bugs.webkit.org/show_bug.cgi?id=135347 to track that in a followup
Created attachment 235611 [details] Patch v1
Comment on attachment 235611 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=235611&action=review > Source/WebKit2/DatabaseProcess/DatabaseProcess.cpp:60 > + // FIXME (<rdar://problem/9127819>): Is there a more explicit way of doing this besides accessing the UTF8Encoding? This should probably be a bugzilla bug. > Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:116 > + if (!callback) { > + LOG_ERROR("WebOriginDataManagerProxy::didGetOrigins - Invalid callback ID"); > + return; > + } > + When can this happen? Should this be a MESSAGE_CHECK()? > Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:131 > + if (!(types & kWKIndexedDatabaseData)) { > + callbackFunction(CallbackBase::Error::None); > return; > + } This should probably have the same comment as above (or just move the one from below). > Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:160 > + if (!(types & kWKIndexedDatabaseData)) { > + callbackFunction(CallbackBase::Error::None); > return; > + } This should probably have the same comment as above (or just move the one from below). > Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:190 > + if (!(types & kWKIndexedDatabaseData)) { > + callbackFunction(CallbackBase::Error::None); > return; > + } This should probably have the same comment as above (or just move the one from below).
(In reply to comment #3) > (From update of attachment 235611 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=235611&action=review > > > Source/WebKit2/DatabaseProcess/DatabaseProcess.cpp:60 > > + // FIXME (<rdar://problem/9127819>): Is there a more explicit way of doing this besides accessing the UTF8Encoding? > > This should probably be a bugzilla bug. Yah, probably! > > Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:116 > > + if (!callback) { > > + LOG_ERROR("WebOriginDataManagerProxy::didGetOrigins - Invalid callback ID"); > > + return; > > + } > > + > > When can this happen? Should this be a MESSAGE_CHECK()? Shouldn't happen. Should probably be a MESSAGE_CHECK. > This should probably have the same comment as above (or just move the one from below). ... > This should probably have the same comment as above (or just move the one from below). ... > This should probably have the same comment as above (or just move the one from below). Willdo
Filed https://bugs.webkit.org/show_bug.cgi?id=135365 for that radar. Will update the comment in the patch (as well as the comment in LocalStorageDatabaseTracker.cpp) before landing.
(In reply to comment #4) > (In reply to comment #3) > > > > Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:116 > > > + if (!callback) { > > > + LOG_ERROR("WebOriginDataManagerProxy::didGetOrigins - Invalid callback ID"); > > > + return; > > > + } > > > + > > > > When can this happen? Should this be a MESSAGE_CHECK()? > > Shouldn't happen. Should probably be a MESSAGE_CHECK. MESSAGE_CHECK requires you know what Connection the current dispatched message came from. Right now, that can only be the DatabaseProcess, so I can do it. In the future, WebOriginDataManagerProxy will *not* know what Connection this message is coming from, so we'll have to deal with that later.
(In reply to comment #6) > (In reply to comment #4) > > (In reply to comment #3) > > > > > > Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:116 > > > > + if (!callback) { > > > > + LOG_ERROR("WebOriginDataManagerProxy::didGetOrigins - Invalid callback ID"); > > > > + return; > > > > + } > > > > + > > > > > > When can this happen? Should this be a MESSAGE_CHECK()? > > > > Shouldn't happen. Should probably be a MESSAGE_CHECK. > > MESSAGE_CHECK requires you know what Connection the current dispatched message came from. > > Right now, that can only be the DatabaseProcess, so I can do it. > > In the future, WebOriginDataManagerProxy will *not* know what Connection this message is coming from, so we'll have to deal with that later. Actually with the magic of WantsConnection, we can do this. New patch forthcoming.
Created attachment 235647 [details] Patch v2 - Also added a new API we'll need very soon anyways
Sorry about the "Also added a new API we'll need very soon anyways" - That got in there from autocomplete history. This V2 is basically V1 with review feedback addressed.
Comment on attachment 235647 [details] Patch v2 - Also added a new API we'll need very soon anyways r=me on the parts of the patch that touch the filesystem.
(In reply to comment #10) > (From update of attachment 235647 [details]) > r=me on the parts of the patch that touch the filesystem. me, too!
http://trac.webkit.org/changeset/171749
(In reply to comment #12) > http://trac.webkit.org/changeset/171749 and the buildfix landed in http://trac.webkit.org/changeset/171753