Bug 156782 - Modern IDB (Workers): Enable INDEXED_DATABASE_IN_WORKERS compile time flag, but disabled in RuntimeEnabledFeatures
Summary: Modern IDB (Workers): Enable INDEXED_DATABASE_IN_WORKERS compile time flag, b...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks: 149117 149953
  Show dependency treegraph
 
Reported: 2016-04-19 22:47 PDT by Brady Eidson
Modified: 2016-04-20 13:21 PDT (History)
14 users (show)

See Also:


Attachments
WIP (23.63 KB, patch)
2016-04-19 22:52 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch v1 (38.32 KB, patch)
2016-04-20 11:02 PDT, Brady Eidson
beidson: review-
Details | Formatted Diff | Diff
Patch v2 (38.28 KB, patch)
2016-04-20 11:03 PDT, Brady Eidson
achristensen: review+
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-04-19 22:47:19 PDT
Modern IDB (Workers): Enable INDEXED_DATABASE_IN_WORKERS compile time flag, but disabled in RuntimeEnabledFeatures
Comment 1 Brady Eidson 2016-04-19 22:52:28 PDT
Created attachment 276805 [details]
WIP

This WIP compiles and is probably almost done, minus a test.
Comment 2 WebKit Commit Bot 2016-04-19 22:54:10 PDT
Attachment 276805 [details] did not pass style-queue:


ERROR: Source/WebKit/mac/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig).  [featuredefines/equality] [5]
ERROR: Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig).  [featuredefines/equality] [5]
ERROR: Source/WebCore/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig).  [featuredefines/equality] [5]
ERROR: Source/WebKit2/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig).  [featuredefines/equality] [5]
Total errors found: 4 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Brady Eidson 2016-04-20 11:02:15 PDT
Created attachment 276834 [details]
Patch v1
Comment 4 Brady Eidson 2016-04-20 11:03:18 PDT
Created attachment 276835 [details]
Patch v2
Comment 5 Alex Christensen 2016-04-20 11:18:33 PDT
Comment on attachment 276835 [details]
Patch v2

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

> LayoutTests/storage/indexeddb/modern/workers-enable-expected.txt:9
> +FAIL [Worker] self.indexedDB should be non-null. Was null
> +FAIL [Worker] self.indexedDB instanceof IDBFactory should be true. Was false.

fail?
Comment 6 Brady Eidson 2016-04-20 11:27:53 PDT
(In reply to comment #5)
> Comment on attachment 276835 [details]
> Patch v2
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=276835&action=review
> 
> > LayoutTests/storage/indexeddb/modern/workers-enable-expected.txt:9
> > +FAIL [Worker] self.indexedDB should be non-null. Was null
> > +FAIL [Worker] self.indexedDB instanceof IDBFactory should be true. Was false.
> 
> fail?

From the ChangeLog:

+        Note: One test has some "FAIL" lines in the expected results, which is intentional.
+        As work on this progresses, those FAILs will become PASSes and the expectations will be updated.
Comment 7 Alex Christensen 2016-04-20 11:32:12 PDT
../../Source/WebCore/testing/InternalSettings.cpp:547:46: error: 'class WebCore::RuntimeEnabledFeatures' has no member named 'setIndexedDBWorkersEnabled'
     RuntimeEnabledFeatures::sharedFeatures().setIndexedDBWorkersEnabled(enabled);

I think you need some more compile time flags.
Comment 8 Brady Eidson 2016-04-20 11:32:58 PDT
(In reply to comment #7)
> ../../Source/WebCore/testing/InternalSettings.cpp:547:46: error: 'class
> WebCore::RuntimeEnabledFeatures' has no member named
> 'setIndexedDBWorkersEnabled'
>     
> RuntimeEnabledFeatures::sharedFeatures().setIndexedDBWorkersEnabled(enabled);
> 
> I think you need some more compile time flags.

Yes, I planned to rely on EWS to tell me where, which is why I didn't cq?
Comment 9 Alex Christensen 2016-04-20 11:34:01 PDT
Comment on attachment 276835 [details]
Patch v2

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

> Source/WebCore/Modules/notifications/WorkerGlobalScopeNotifications.cpp:59
> -        provideTo(context, supplementName(), WTFMove(newSupplement));
> +        provideTo(&scope, supplementName(), WTFMove(newSupplement));

Outside the scope of this patch, but we should make provideTo take a reference.
Comment 10 Brady Eidson 2016-04-20 11:35:34 PDT
(In reply to comment #9)
> Comment on attachment 276835 [details]
> Patch v2
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=276835&action=review
> 
> > Source/WebCore/Modules/notifications/WorkerGlobalScopeNotifications.cpp:59
> > -        provideTo(context, supplementName(), WTFMove(newSupplement));
> > +        provideTo(&scope, supplementName(), WTFMove(newSupplement));
> 
> Outside the scope of this patch, but we should make provideTo take a
> reference.

Considered doing it in this patch before it become obvious just how many sites would have to change.
Comment 11 Brady Eidson 2016-04-20 12:01:59 PDT
http://trac.webkit.org/changeset/199779
Comment 12 Csaba Osztrogonác 2016-04-20 13:15:05 PDT
(In reply to comment #11)
> http://trac.webkit.org/changeset/199779

It broke the non INDEXED_DATABASE_IN_WORKERS build, for example EFL and GTK. And not in Webkit2, but in Webcore.
Comment 13 Brady Eidson 2016-04-20 13:15:51 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > http://trac.webkit.org/changeset/199779
> 
> It broke the non INDEXED_DATABASE_IN_WORKERS build, for example EFL and GTK.
> And not in Webkit2, but in Webcore.

Link to failure? I saw that breakage in EWS and attempted to fix.
Comment 14 Brady Eidson 2016-04-20 13:16:56 PDT
A link to the failure so others don't have to go hunting:
https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Build%29/builds/68616/steps/compile-webkit/logs/stdio

Working on a fix.
Comment 15 Brady Eidson 2016-04-20 13:21:24 PDT
Fix attempt in http://trac.webkit.org/changeset/199782