Bug 157823 - Modern IDB: Make TestRunner.clearAllDatabases also delete IndexedDB databases (once doing so is supported)
Summary: Modern IDB: Make TestRunner.clearAllDatabases also delete IndexedDB databases...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks: 149117 157626
  Show dependency treegraph
 
Reported: 2016-05-17 17:16 PDT by Brady Eidson
Modified: 2022-02-28 04:10 PST (History)
5 users (show)

See Also:


Attachments
Patch v1 (12.86 KB, patch)
2016-05-17 17:20 PDT, Brady Eidson
achristensen: review+
Details | Formatted Diff | Diff
Patch for ES (13.82 KB, application/octet-stream)
2016-05-18 08:52 PDT, Brady Eidson
no flags Details
Patch for EWS + landing (13.82 KB, patch)
2016-05-18 08:52 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2016-05-17 17:16:39 PDT
Modern IDB: Make TestRunner.clearAllDatabases also delete IndexedDB databases (once doing so is supported in WebCore)

Landing this separately is an attempt to make the work in https://bugs.webkit.org/show_bug.cgi?id=157626 more manageable to polish and review.
Comment 1 Brady Eidson 2016-05-17 17:20:36 PDT
Created attachment 279188 [details]
Patch v1
Comment 2 WebKit Commit Bot 2016-05-17 17:22:57 PDT
Attachment 279188 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/server/IDBServer.h:99:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:471:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Alex Christensen 2016-05-17 23:05:25 PDT
Comment on attachment 279188 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=279188&action=review

> Source/WebKit2/UIProcess/API/C/WKWebsiteDataStoreRef.cpp:62
> +void WKWebsiteDataStoreRemoveAllIndexedDatabases(WKWebsiteDataStoreRef dataStoreRef)
> +{
> +    OptionSet<WebKit::WebsiteDataType> dataTypes = WebKit::WebsiteDataType::IndexedDBDatabases;
> +    return WebKit::toImpl(dataStoreRef)->websiteDataStore().removeData(dataTypes, std::chrono::system_clock::time_point::min(), [] { });
> +}

return??? If I wrote my own c++ compiler, this would be a compiler error.

> Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:259
> +    WKBundlePostSynchronousMessage(InjectedBundle::singleton().bundle(), messageName.get(), messageBody.get(), 0);

nullptr

> Tools/WebKitTestRunner/TestInvocation.cpp:719
> +        WKWebsiteDataStoreRemoveAllIndexedDatabases(WKContextGetWebsiteDataStore(TestController::singleton().context()));

EFL doesn't like this.  I'm not sure why.  Everything seems correct.  Maybe they're including an old version of the header without the change.
WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle(OpaqueWKString const*, void const*): error: undefined reference to 'WKWebsiteDataStoreRemoveAllIndexedDatabases'
Comment 4 Brady Eidson 2016-05-18 08:50:13 PDT
(In reply to comment #3)
> Comment on attachment 279188 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=279188&action=review
> 
> > Source/WebKit2/UIProcess/API/C/WKWebsiteDataStoreRef.cpp:62
> > +void WKWebsiteDataStoreRemoveAllIndexedDatabases(WKWebsiteDataStoreRef dataStoreRef)
> > +{
> > +    OptionSet<WebKit::WebsiteDataType> dataTypes = WebKit::WebsiteDataType::IndexedDBDatabases;
> > +    return WebKit::toImpl(dataStoreRef)->websiteDataStore().removeData(dataTypes, std::chrono::system_clock::time_point::min(), [] { });
> > +}
> 
> return??? If I wrote my own c++ compiler, this would be a compiler error.

"void" is a valid return in a void function. :)

(I copied this line from somewhere else before editing it)

> > Tools/WebKitTestRunner/TestInvocation.cpp:719
> > +        WKWebsiteDataStoreRemoveAllIndexedDatabases(WKContextGetWebsiteDataStore(TestController::singleton().context()));
> 
> EFL doesn't like this.  I'm not sure why.  Everything seems correct.  Maybe
> they're including an old version of the header without the change.
> WTR::TestInvocation::
> didReceiveSynchronousMessageFromInjectedBundle(OpaqueWKString const*, void
> const*): error: undefined reference to
> 'WKWebsiteDataStoreRemoveAllIndexedDatabases'

They never compiled WKWebsiteDataStoreRef. They will now.
Comment 5 Brady Eidson 2016-05-18 08:52:34 PDT
Created attachment 279255 [details]
Patch for ES
Comment 6 Brady Eidson 2016-05-18 08:52:54 PDT
Created attachment 279256 [details]
Patch for EWS + landing
Comment 7 WebKit Commit Bot 2016-05-18 08:53:51 PDT
Attachment 279256 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/server/IDBServer.h:99:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:471:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 WebKit Commit Bot 2016-05-18 09:29:32 PDT
Comment on attachment 279256 [details]
Patch for EWS + landing

Clearing flags on attachment: 279256

Committed r201081: <http://trac.webkit.org/changeset/201081>