Currently the commands that we generate (InspectorFooBackendCommands.js) include all domains, without checking whether their feature flags are actually enabled in the build system. This is fine for C++ code where we can just wrap the relevant agents and type builders in ENABLE(FOO_FEATURE). This blocks frontend feature detection for ENABLE(WEB_REPLAY), which is not enabled by default. We want to support it being either on or off, but right now the frontend will always think it's available, whether or not the ReplayAgent backend has been compiled or not.
Using the C preprocessor to filter things out of the JSON files and prevent generation sounds fine to me.
Joe suggested an even easier solution: conditionally add protocol spec files inside DerivedSources.make. I am going to wait to submit this until 130110 because it has no effect presently (the ruby script to update doesn't take into account ENABLE flags).
Created attachment 226759 [details] the patch
<rdar://problem/16332612>
Comment on attachment 226759 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=226759&action=review > Source/WebCore/CMakeLists.txt:765 > inspector/protocol/IndexedDB.json Same for IndexedDB as Databases? > Source/WebCore/GNUmakefile.am:374 > +EnabledInspectorDomains: force > + echo '$(WebCore_INSPECTOR_DOMAINS)' | cmp -s - $@ || echo '$(WebCore_INSPECTOR_DOMAINS)' > $@ > + > DerivedSources/WebCore/InspectorWeb.json: $(INSPECTOR_SCRIPTS_DIR)/generate-combined-inspector-json.py $(WebCore_INSPECTOR_DOMAINS) Doesn't InspectorWeb.json need to have "EnabledInspectorDomains" as a dependency?
Created attachment 226792 [details] patch round 2
Comment on attachment 226792 [details] patch round 2 This breaks the generation of InspectorWeb.json on GTK at least. And it screws up the following builds on the GTK EWS, at least..
Comment on attachment 226792 [details] patch round 2 View in context: https://bugs.webkit.org/attachment.cgi?id=226792&action=review > Source/WebCore/CMakeLists.txt:2604 > + inspector/protocol/Database.json This line needs to be ${WEBCORE_DIR}/inspector/protocol/Database.json
(In reply to comment #5) > (From update of attachment 226759 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226759&action=review > > > Source/WebCore/CMakeLists.txt:765 > > inspector/protocol/IndexedDB.json > > Same for IndexedDB as Databases? nothing else is conditional on ENABLE_INDEXED_DATABASE so I didn't bother. > > > Source/WebCore/GNUmakefile.am:374 > > +EnabledInspectorDomains: force > > + echo '$(WebCore_INSPECTOR_DOMAINS)' | cmp -s - $@ || echo '$(WebCore_INSPECTOR_DOMAINS)' > $@ > > + > > DerivedSources/WebCore/InspectorWeb.json: $(INSPECTOR_SCRIPTS_DIR)/generate-combined-inspector-json.py $(WebCore_INSPECTOR_DOMAINS) > > Doesn't InspectorWeb.json need to have "EnabledInspectorDomains" as a dependency? Yes.
(In reply to comment #8) > (From update of attachment 226792 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226792&action=review > > > Source/WebCore/CMakeLists.txt:2604 > > + inspector/protocol/Database.json > > This line needs to be ${WEBCORE_DIR}/inspector/protocol/Database.json Oops, I should have changed that one too. Thanks.
Created attachment 226846 [details] patch round 3
Ooops, forgot that we can't guard generating WEB_REPLAY on mac builds because Xcode doesn't like that. Will remove that before landing.
Committed r165714: <http://trac.webkit.org/changeset/165714>