Bug 49500 - Buildfix for !ENABLE(INSPECTOR)
Summary: Buildfix for !ENABLE(INSPECTOR)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Patrick R. Gansterer
URL:
Keywords:
: 49229 49462 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-11-13 15:28 PST by Patrick R. Gansterer
Modified: 2010-11-15 08:55 PST (History)
7 users (show)

See Also:


Attachments
Patch (3.65 KB, patch)
2010-11-13 15:36 PST, Patrick R. Gansterer
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick R. Gansterer 2010-11-13 15:28:00 PST
see patch
Comment 1 Patrick R. Gansterer 2010-11-13 15:36:34 PST
Created attachment 73833 [details]
Patch
Comment 2 Kwang Yul Seo 2010-11-13 16:09:50 PST
*** Bug 49229 has been marked as a duplicate of this bug. ***
Comment 3 Yury Semikhatsky 2010-11-13 23:37:16 PST
Comment on attachment 73833 [details]
Patch

Thanks for fixing this. The problem must have appeared after http://trac.webkit.org/changeset/71515 where I reasoned that ENABLED(INSPECTOR) guards can be removed from the ConsoleMessage.cpp because the latter is only used by the inspector code itself.  I confined my check to compilation of --minimal build which is usually enough to see if there are any issues related to ENABLED(INSPECTOR) macro.
Comment 4 WebKit Commit Bot 2010-11-13 23:57:04 PST
Comment on attachment 73833 [details]
Patch

Clearing flags on attachment: 73833

Committed r71981: <http://trac.webkit.org/changeset/71981>
Comment 5 WebKit Commit Bot 2010-11-13 23:57:09 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 Csaba Osztrogonác 2010-11-14 02:18:04 PST
(In reply to comment #3)
> (From update of attachment 73833 [details])
> Thanks for fixing this. The problem must have appeared after http://trac.webkit.org/changeset/71515 where I reasoned that ENABLED(INSPECTOR) guards can be removed from the ConsoleMessage.cpp because the latter is only used by the inspector code itself.  I confined my check to compilation of --minimal build which is usually enough to see if there are any issues related to ENABLED(INSPECTOR) macro.

--minimal option disables features defined in @features in build-webkit script,
but INSPECTOR isn't in it. Do we need to it to the build systems? I checked ENABLE_INSPECTOR guards and only PLATFORM(IOS) (in Platform.h) and PLATFORM(ANDROID) (in WebCore/config.h) disable it.

To add/remove a feature is so complex task, because all build system have its own default value for all features. And there are platform specific default values in build-webkit script, Platform.h, JavascriptCore/config.h and WebCore/config.h . See http://trac.webkit.org/changeset/71754 for details and https://bugs.webkit.org/show_bug.cgi?id=48755 why is so complex to modify MSVC build files.
Comment 7 Andreas Kling 2010-11-14 11:02:37 PST
*** Bug 49462 has been marked as a duplicate of this bug. ***
Comment 8 Joseph Pecoraro 2010-11-15 08:42:49 PST
Its a little weird having just the implementation of ScriptCallStack::buildInspectorObject being
ifdef'd out, but not the signature in the .h. I think the entire file could have been guarded, but
this works fine. Thanks for fixing this.
Comment 9 Patrick R. Gansterer 2010-11-15 08:48:09 PST
(In reply to comment #8)
> Its a little weird having just the implementation of ScriptCallStack::buildInspectorObject being ifdef'd out, but not the signature in the .h.
bug 49229#c2 says:
> The approach so far has been to flag the dependencies only in the body so that the flag does not bleed over. I'd like to continue this approach.

> I think the entire file could have been guarded, but this works fine.
You can't ifdef the whole Script*.h files, because there are dependencies in Console.cpp.
Comment 10 Joseph Pecoraro 2010-11-15 08:55:24 PST
Good to know. Thanks!