If you disable inspector in Platform.h via #define ENABLE_INSPECTOR 0 build will fail ultimately, mostly because the build system does not handle this case. Disabling such features are important for embedded systems.
Btw fixing the sources to handle !ENABLE(INSPECTOR) case is easy but the build system has inspector support hardcoded.
Created attachment 45636 [details] proposed patch To turn off inspector support built QtWebKit with the following command build-webkit --no-javascript-debugger DEFINES+="ENABLE_INSPECTOR=0"
style-queue ran check-webkit-style on attachment 45636 [details] without any errors.
Comment on attachment 45636 [details] proposed patch Should InspectElement even be defined? 20372045 case InspectElement: { Seems we might as well remove that when ENABLE(INSPECTOR) isn't defined (unless it's part of some public API header and difficutl to remove?) In general this looks OK. Why would we even compile InspectorClientQt when ENABLE(INSPECTOR) was not defined?
Also --minimal should disable inspector too.
Comment on attachment 45636 [details] proposed patch r=me. I wonder what the gain is for embedded builds, given that we exclude the bulk of data files anyway.
Well even 10k matters for me on Windows CE 5.0. Thanks for the patch Laszlo, its appreciated.
Comment on attachment 45636 [details] proposed patch Clearing flags on attachment: 45636 Committed r52662: <http://trac.webkit.org/changeset/52662>
All reviewed patches have been landed. Closing bug.
The patch misses some ifdefs it seems, I get: ..\..\..\WebCore\rendering\RenderLayerBacking.cpp(1028) : error C2039: 'inspectorTimelineAgent' : is not a member of 'WebCore::Page' see declaration of 'WebCore::Page'
Created attachment 48238 [details] Patch to add missing ifdef for the disabled inspector case
Created attachment 48239 [details] Patch to add missing ifdef for the disabled inspector case (add the semicolon to make it compile!)
Ping?
Created attachment 48795 [details] Patch against latest SVN, fix new compilation failures
Comment on attachment 48795 [details] Patch against latest SVN, fix new compilation failures > > static InspectorTimelineAgent* inspectorTimelineAgent(RenderObject* renderer) > { > +#if !ENABLE(INSPECTOR) > + return 0; > +#else > Frame* frame = renderer->document()->frame(); > if (!frame) > return 0; > @@ -1026,6 +1029,7 @@ static InspectorTimelineAgent* inspectorTimelineAgent(RenderObject* renderer) > if (!page) > return 0; > return page->inspectorTimelineAgent(); > +#endif > } You should just surround this entire method with if ENABLE(INSPECTOR). Please fix this before landing. > > // Up-call from compositing layer drawing callback. > diff --git a/WebKit/qt/Api/qwebpage.cpp b/WebKit/qt/Api/qwebpage.cpp > index 185a2ee..256d6c4 100644 > --- a/WebKit/qt/Api/qwebpage.cpp > +++ b/WebKit/qt/Api/qwebpage.cpp > @@ -163,27 +163,42 @@ QString QWEBKIT_EXPORT qt_webpage_groupName(QWebPage* page) > > void QWEBKIT_EXPORT qt_drt_webinspector_executeScript(QWebPage* page, long callId, const QString& script) > { Nit: In case these are only used from within DRT, should you hide these methods as well and use guards in layout test controller instead?
(In reply to comment #15) > (From update of attachment 48795 [details]) > > > > static InspectorTimelineAgent* inspectorTimelineAgent(RenderObject* renderer) > > { > > +#if !ENABLE(INSPECTOR) > > + return 0; > > +#else > > Frame* frame = renderer->document()->frame(); > > if (!frame) > > return 0; > > @@ -1026,6 +1029,7 @@ static InspectorTimelineAgent* inspectorTimelineAgent(RenderObject* renderer) > > if (!page) > > return 0; > > return page->inspectorTimelineAgent(); > > +#endif > > } > > > You should just surround this entire method with if ENABLE(INSPECTOR). Please > fix this before landing. Since I cannot land myself anyway, if I just surround with if ENABLE(INSPECTOR) it would give a compiler warning/error saying the function should return a value.
(In reply to comment #16) > (In reply to comment #15) > > (From update of attachment 48795 [details] [details]) > > > > > > static InspectorTimelineAgent* inspectorTimelineAgent(RenderObject* renderer) > > > { > > > +#if !ENABLE(INSPECTOR) > > > + return 0; > > > +#else > > > Frame* frame = renderer->document()->frame(); > > > if (!frame) > > > return 0; > > > @@ -1026,6 +1029,7 @@ static InspectorTimelineAgent* inspectorTimelineAgent(RenderObject* renderer) > > > if (!page) > > > return 0; > > > return page->inspectorTimelineAgent(); > > > +#endif > > > } > > > > > > You should just surround this entire method with if ENABLE(INSPECTOR). Please > > fix this before landing. > > Since I cannot land myself anyway, if I just surround with if ENABLE(INSPECTOR) > it would give a compiler warning/error saying the function should return a > value. I mean surround the function, not its body. It should never be referenced if there is no inspector.
(In reply to comment #17) > (In reply to comment #16) > > (In reply to comment #15) > > > (From update of attachment 48795 [details] [details] [details]) > > > > > > > > static InspectorTimelineAgent* inspectorTimelineAgent(RenderObject* renderer) > > > > { > > > > +#if !ENABLE(INSPECTOR) > > > > + return 0; > > > > +#else > > > > Frame* frame = renderer->document()->frame(); > > > > if (!frame) > > > > return 0; > > > > @@ -1026,6 +1029,7 @@ static InspectorTimelineAgent* inspectorTimelineAgent(RenderObject* renderer) > > > > if (!page) > > > > return 0; > > > > return page->inspectorTimelineAgent(); > > > > +#endif > > > > } > > > > > > > > > You should just surround this entire method with if ENABLE(INSPECTOR). Please > > > fix this before landing. > > > > Since I cannot land myself anyway, if I just surround with if ENABLE(INSPECTOR) > > it would give a compiler warning/error saying the function should return a > > value. > > I mean surround the function, not its body. It should never be referenced if > there is no inspector. That would require patching the header too, isn't that kind of ugly? :-)
> That would require patching the header too, isn't that kind of ugly? :-) There is no (should be no) header entry for it. It is just a static helper function defined for private use in this cpp.
(In reply to comment #19) > > That would require patching the header too, isn't that kind of ugly? :-) > > There is no (should be no) header entry for it. It is just a static helper > function defined for private use in this cpp. I forgot the fact that the function was static, was lame of me. Let me upload a fixed patch.
Created attachment 48797 [details] Surround the functions with a ifdef block, this is simpler
Comment on attachment 48797 [details] Surround the functions with a ifdef block, this is simpler What about static InspectorTimelineAgent* inspectorTimelineAgent It is still not hidden, while it was the only one I really cared about.
Created attachment 48798 [details] Simplify the patch
Comment on attachment 48798 [details] Simplify the patch Clearing flags on attachment: 48798 Committed r54815: <http://trac.webkit.org/changeset/54815>