| Summary: | REGRESSION(r178527): It broke the !ENABLE(INSPECTOR) build | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Csaba Osztrogonác <ossy> |
| Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED WONTFIX | ||
| Severity: | Critical | CC: | ap, burg, joepeck, ossy, timothy |
| Priority: | P1 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Bug Depends on: | |||
| Bug Blocks: | 140478 | ||
Does r178527 mean that inspector is mandatory feature since now? If yes, let's remove the compile time guards and make it really mandatory. If no, could you possibly give me any hint how to fix the build properly? Given that we use the logic in Inspector::ConsoleMessage even when inspector is not enabled, yes, this logic needs to be always compiled in. I haven't looked into how much of other Inspector code it uses. Perhaps it can be factored out of Inspector::ConsoleMessage. What is the rationale for disabling Inspector? The !ENABLE(INSPECTOR) build appears to be very high maintenance. (In reply to comment #1) > Does r178527 mean that inspector is mandatory feature since now? > > If yes, let's remove the compile time guards and make it really mandatory. > If no, could you possibly give me any hint how to fix the build properly? It seems that these are the only methods that actually require ENABLE(INSPECTOR), and are entry points from InspectorConsoleAgent: void addToFrontend(InspectorConsoleFrontendDispatcher*, InjectedScriptManager*, bool generatePreview); void updateRepeatCountInConsole(InspectorConsoleFrontendDispatcher*); Plus IdentifiersFactory.h which looks to have no other Inspector dependencies. So you would need to remove the guard of the entire file and only surround those methods (and their static helper functions in the .cpp file). Honestly, someone should request to webkit-dev to remove ENABLE(INSPECTOR) guards. There are no bots running this configuration, and I would be surprised if anyone contributing to trunk depends on it. If such a person exists, I would have expected to hear about these build breaks before Ossy found it via --minimal. (In reply to comment #2) > Given that we use the logic in Inspector::ConsoleMessage even when inspector > is not enabled, yes, this logic needs to be always compiled in. I haven't > looked into how much of other Inspector code it uses. Perhaps it can be > factored out of Inspector::ConsoleMessage. > > What is the rationale for disabling Inspector? The !ENABLE(INSPECTOR) build > appears to be very high maintenance. It can be useful to reduce the binary codesize on embedded systems where you don't need inspector at all. ( https://bugs.webkit.org/show_bug.cgi?id=140098#c5 ) I don't have strong opinion about it, but I don't like broken and unmaintained ifdef guards. If we want to have a guard, we should maintain it, if we don't want to maintain it, let's remove the guard. (In reply to comment #3) > Honestly, someone should request to webkit-dev to remove ENABLE(INSPECTOR) > guards. There are no bots running this configuration, and I would be > surprised if anyone contributing to trunk depends on it. If such a person > exists, I would have expected to hear about these build breaks before Ossy > found it via --minimal. Will send soon. webkit-dev mail sent: https://lists.webkit.org/pipermail/webkit-dev/2015-January/027153.html There is no !INSPECTOR build since http://trac.webkit.org/changeset/178820 . |
../../Source/WebCore/page/PageConsoleClient.cpp: In member function 'void WebCore::PageConsoleClient::addMessage(JSC::MessageSource, JSC::MessageLevel, const WTF::String&, const WTF::String&, unsigned int, unsigned int, WTF::RefPtr<Inspe ctor::ScriptCallStack>&&, JSC::ExecState*, long unsigned int)': ../../Source/WebCore/page/PageConsoleClient.cpp:127:25: error: invalid use of incomplete type 'class Inspector::ConsoleMessage' In file included from ../../Source/WebCore/inspector/InspectorConsoleInstrumentation.h:34:0, from ../../Source/WebCore/page/PageConsoleClient.cpp:36: ../../Source/WebCore/inspector/InspectorInstrumentation.h:60:7: error: forward declaration of 'class Inspector::ConsoleMessage' ../../Source/WebCore/page/PageConsoleClient.cpp:128:34: error: invalid use of incomplete type 'class Inspector::ConsoleMessage' In file included from ../../Source/WebCore/inspector/InspectorConsoleInstrumentation.h:34:0, from ../../Source/WebCore/page/PageConsoleClient.cpp:36: ../../Source/WebCore/inspector/InspectorInstrumentation.h:60:7: error: forward declaration of 'class Inspector::ConsoleMessage' ../../Source/WebCore/page/PageConsoleClient.cpp:129:36: error: invalid use of incomplete type 'class Inspector::ConsoleMessage' In file included from ../../Source/WebCore/inspector/InspectorConsoleInstrumentation.h:34:0, from ../../Source/WebCore/page/PageConsoleClient.cpp:36: ../../Source/WebCore/inspector/InspectorInstrumentation.h:60:7: error: forward declaration of 'class Inspector::ConsoleMessage' ... The whole ConsoleMessage.h is guarded with ENABLE(INSPECTOR), but it is ununsed without guard in PageConsoleClient.cpp.