[webkit-reviews] review granted: [Bug 29260] Add ENABLE(INSPECTOR) : [Attachment 39577] This patch implements the needed changes
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Sep 16 11:56:34 PDT 2009
David Kilzer (ddkilzer) <ddkilzer at webkit.org> has granted Greg Bolsinga
<bolsinga at apple.com>'s request for review:
Bug 29260: Add ENABLE(INSPECTOR)
https://bugs.webkit.org/show_bug.cgi?id=29260
Attachment 39577: This patch implements the needed changes
https://bugs.webkit.org/attachment.cgi?id=39577&action=review
------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
> diff --git a/WebCore/bindings/js/JSInspectedObjectWrapper.cpp
b/WebCore/bindings/js/JSInspectedObjectWrapper.cpp
> index fff7aee..dc3e5dc 100644
> --- a/WebCore/bindings/js/JSInspectedObjectWrapper.cpp
> +++ b/WebCore/bindings/js/JSInspectedObjectWrapper.cpp
> @@ -26,6 +26,7 @@
> #include "config.h"
> #include "JSInspectedObjectWrapper.h"
>
> +#if ENABLE(INSPECTOR)
> #include "JSInspectorCallbackWrapper.h"
> #include <runtime/JSGlobalObject.h>
> #include <wtf/StdLibExtras.h>
> @@ -125,3 +126,4 @@ JSValue
JSInspectedObjectWrapper::prepareIncomingValue(ExecState*, JSValue value
> }
>
> } // namespace WebCore
> +#endif // ENABLE(INSPECTOR)
Please add a blank line after the "#if ENABLE(.." statement and before the
"#endif // ..." statement.
> diff --git a/WebCore/bindings/js/JSInspectorBackendCustom.cpp
b/WebCore/bindings/js/JSInspectorBackendCustom.cpp
> index 782ffab..f62aecf 100644
> --- a/WebCore/bindings/js/JSInspectorBackendCustom.cpp
> +++ b/WebCore/bindings/js/JSInspectorBackendCustom.cpp
> @@ -33,6 +33,7 @@
> #include "config.h"
> #include "JSInspectorBackend.h"
>
> +#if ENABLE(INSPECTOR)
> #include "Console.h"
> #if ENABLE(DATABASE)
> #include "Database.h"
> @@ -364,3 +365,4 @@ JSValue JSInspectorBackend::selectDOMStorage(ExecState*,
const ArgList& args)
> #endif
>
> } // namespace WebCore
> +#endif // ENABLE(INSPECTOR)
Ditto.
> diff --git a/WebCore/bindings/js/JSInspectorCallbackWrapper.cpp
b/WebCore/bindings/js/JSInspectorCallbackWrapper.cpp
> index 5c3cd29..e69aeaa 100644
> --- a/WebCore/bindings/js/JSInspectorCallbackWrapper.cpp
> +++ b/WebCore/bindings/js/JSInspectorCallbackWrapper.cpp
> @@ -26,6 +26,7 @@
> #include "config.h"
> #include "JSInspectorCallbackWrapper.h"
>
> +#if ENABLE(INSPECTOR)
> #include "JSInspectedObjectWrapper.h"
> #include <wtf/StdLibExtras.h>
>
> @@ -105,3 +106,4 @@ JSValue
JSInspectorCallbackWrapper::prepareIncomingValue(ExecState* unwrappedExe
> }
>
> } // namespace WebCore
> +#endif // ENABLE(INSPECTOR)
Ditto.
> diff --git a/WebCore/bindings/js/ScriptObjectQuarantine.cpp
b/WebCore/bindings/js/ScriptObjectQuarantine.cpp
> index f96f89e..10a61c5 100644
> --- a/WebCore/bindings/js/ScriptObjectQuarantine.cpp
> +++ b/WebCore/bindings/js/ScriptObjectQuarantine.cpp
> @@ -31,6 +31,7 @@
> #include "config.h"
> #include "ScriptObjectQuarantine.h"
>
> +#if ENABLE(INSPECTOR)
> #include "Document.h"
> #include "Frame.h"
> #include "JSDOMBinding.h"
Ditto.
> diff --git a/WebCore/inspector/DOMDispatchTimelineItem.cpp
b/WebCore/inspector/DOMDispatchTimelineItem.cpp
> index 59c4ca5..9b6fd46 100644
> --- a/WebCore/inspector/DOMDispatchTimelineItem.cpp
> +++ b/WebCore/inspector/DOMDispatchTimelineItem.cpp
> @@ -31,6 +31,7 @@
> #include "config.h"
> #include "DOMDispatchTimelineItem.h"
>
> +#if ENABLE(INSPECTOR)
> #include "Event.h"
> #include "InspectorFrontend.h"
>
Ditto.
> diff --git a/WebCore/inspector/InspectorBackend.cpp
b/WebCore/inspector/InspectorBackend.cpp
> index 6d82109..42b4b4d 100644
> --- a/WebCore/inspector/InspectorBackend.cpp
> +++ b/WebCore/inspector/InspectorBackend.cpp
> @@ -30,6 +30,7 @@
> #include "config.h"
> #include "InspectorBackend.h"
>
> +#if ENABLE(INSPECTOR)
> #if ENABLE(DATABASE)
> #include "Database.h"
> #endif
> @@ -520,3 +521,4 @@ InspectorFrontend* InspectorBackend::inspectorFrontend()
> }
>
> } // namespace WebCore
> +#endif // ENABLE(INSPECTOR)
Ditto.
> diff --git a/WebCore/inspector/InspectorController.cpp
b/WebCore/inspector/InspectorController.cpp
> index a5e8184..1415258 100644
> --- a/WebCore/inspector/InspectorController.cpp
> +++ b/WebCore/inspector/InspectorController.cpp
> @@ -30,6 +30,7 @@
> #include "config.h"
> #include "InspectorController.h"
>
> +#if ENABLE(INSPECTOR)
> #include "CString.h"
> #include "CachedResource.h"
> #include "Console.h"
> @@ -1564,3 +1565,4 @@ void InspectorController::deleteCookie(const String&
cookieName)
> }
>
> } // namespace WebCore
> +#endif // ENABLE(INSPECTOR)
Ditto.
> diff --git a/WebCore/inspector/InspectorDOMAgent.cpp
b/WebCore/inspector/InspectorDOMAgent.cpp
> index fa25e4d..759d840 100644
> --- a/WebCore/inspector/InspectorDOMAgent.cpp
> +++ b/WebCore/inspector/InspectorDOMAgent.cpp
> @@ -31,6 +31,7 @@
> #include "config.h"
> #include "InspectorDOMAgent.h"
>
> +#if ENABLE(INSPECTOR)
> #include "AtomicString.h"
> #include "ContainerNode.h"
> #include "Cookie.h"
> @@ -547,3 +548,4 @@ Document* InspectorDOMAgent::mainFrameDocument()
> }
>
> } // namespace WebCore
> +#endif // ENABLE(INSPECTOR)
Ditto.
> diff --git a/WebCore/inspector/InspectorDOMStorageResource.cpp
b/WebCore/inspector/InspectorDOMStorageResource.cpp
> index 1e56742..9278afb 100644
> --- a/WebCore/inspector/InspectorDOMStorageResource.cpp
> +++ b/WebCore/inspector/InspectorDOMStorageResource.cpp
> @@ -30,6 +30,7 @@
>
> #include "config.h"
> #if ENABLE(DOM_STORAGE)
> +#if ENABLE(INSPECTOR)
This should be changed to:
#if ENABLE(DOM_STORAGE) && ENABLE(INSPECTOR)
> @@ -80,4 +81,5 @@ void InspectorDOMStorageResource::unbind()
>
> } // namespace WebCore
>
> +#endif // ENABLE(INSPECTOR)
> #endif
These #endif statements should be combined:
#endif // ENABLE(DOM_STORAGE) && ENABLE(INSPECTOR)
> diff --git a/WebCore/inspector/InspectorDatabaseResource.cpp
b/WebCore/inspector/InspectorDatabaseResource.cpp
> index fba50a5..2a78e34 100644
> --- a/WebCore/inspector/InspectorDatabaseResource.cpp
> +++ b/WebCore/inspector/InspectorDatabaseResource.cpp
> @@ -32,6 +32,7 @@
> #if ENABLE(DATABASE)
> #include "InspectorDatabaseResource.h"
>
> +#if ENABLE(INSPECTOR)
> #include "Database.h"
> #include "Document.h"
> #include "Frame.h"
> @@ -75,4 +76,5 @@ void InspectorDatabaseResource::unbind()
>
> } // namespace WebCore
>
> +#endif // ENABLE(INSPECTOR)
> #endif // ENABLE(DATABASE)
I think these #if/#endif statements can be combined as well. See previous
source changes.
> diff --git a/WebCore/inspector/InspectorFrontend.cpp
b/WebCore/inspector/InspectorFrontend.cpp
> index bb369bd..c5a3a3b 100644
> --- a/WebCore/inspector/InspectorFrontend.cpp
> +++ b/WebCore/inspector/InspectorFrontend.cpp
> @@ -30,6 +30,7 @@
> #include "config.h"
> #include "InspectorFrontend.h"
>
> +#if ENABLE(INSPECTOR)
> #include "ConsoleMessage.h"
> #include "Frame.h"
> #include "InspectorController.h"
> @@ -430,3 +431,4 @@ void InspectorFrontend::callSimpleFunction(const String&
functionName)
> }
>
> } // namespace WebCore
> +#endif // ENABLE(INSPECTOR)
Please add a blank line after the "#if ENABLE(.." statement and before the
"#endif // ..." statement.
> diff --git a/WebCore/inspector/InspectorResource.cpp
b/WebCore/inspector/InspectorResource.cpp
> index f4d9ed2..6c8a6ad 100644
> --- a/WebCore/inspector/InspectorResource.cpp
> +++ b/WebCore/inspector/InspectorResource.cpp
> @@ -31,6 +31,7 @@
> #include "config.h"
> #include "InspectorResource.h"
>
> +#if ENABLE(INSPECTOR)
> #include "CachedResource.h"
> #include "DocLoader.h"
> #include "DocumentLoader.h"
> @@ -330,3 +331,4 @@ void InspectorResource::addLength(int lengthReceived)
> }
>
> } // namespace WebCore
> +#endif // ENABLE(INSPECTOR)
Ditto.
> diff --git a/WebCore/inspector/InspectorTimelineAgent.cpp
b/WebCore/inspector/InspectorTimelineAgent.cpp
> index e9d20e0..b221e80 100644
> --- a/WebCore/inspector/InspectorTimelineAgent.cpp
> +++ b/WebCore/inspector/InspectorTimelineAgent.cpp
> @@ -31,6 +31,7 @@
> #include "config.h"
> #include "InspectorTimelineAgent.h"
>
> +#if ENABLE(INSPECTOR)
> #include "DOMDispatchTimelineItem.h"
> #include "Event.h"
> #include "InspectorFrontend.h"
> @@ -136,3 +137,4 @@ double
InspectorTimelineAgent::sessionTimeInMilliseconds()
> }
>
> } // namespace WebCore
> +#endif // ENABLE(INSPECTOR)
Ditto.
> diff --git a/WebCore/inspector/TimelineItem.cpp
b/WebCore/inspector/TimelineItem.cpp
> index 05b81b4..c3e1170 100644
> --- a/WebCore/inspector/TimelineItem.cpp
> +++ b/WebCore/inspector/TimelineItem.cpp
> @@ -31,6 +31,7 @@
> #include "config.h"
> #include "TimelineItem.h"
>
> +#if ENABLE(INSPECTOR)
> #include "InspectorFrontend.h"
> #include "ScriptArray.h"
> #include "ScriptObject.h"
> @@ -76,3 +77,4 @@ void TimelineItem::addChildItem(PassOwnPtr<TimelineItem>
timelineItem)
>
> } // namespace WebCore
>
> +#endif // ENABLE(INSPECTOR)
Ditto.
> diff --git a/WebCore/page/ContextMenuController.cpp
b/WebCore/page/ContextMenuController.cpp
> index 0ec9d1c..a3cb9c7 100644
> --- a/WebCore/page/ContextMenuController.cpp
> +++ b/WebCore/page/ContextMenuController.cpp
> @@ -325,10 +327,12 @@ void
ContextMenuController::contextMenuItemSelected(ContextMenuItem* item)
>
frame->editor()->changeBackToReplacedString(result.replacedString());
> break;
> #endif
> +#if ENABLE(INSPECTOR)
> case ContextMenuItemTagInspectElement:
> if (Page* page = frame->page())
>
page->inspectorController()->inspect(result.innerNonSharedNode());
> break;
> +#endif
> default:
> break;
> }
The #if/#endif should go inside the case statement (but before the break
statement).
> diff --git a/WebKit/mac/ChangeLog b/WebKit/mac/ChangeLog
> diff --git a/WebKit/mac/WebCoreSupport/WebInspectorClient.mm
b/WebKit/mac/WebCoreSupport/WebInspectorClient.mm
> diff --git a/WebKit/mac/WebInspector/WebInspector.mm
b/WebKit/mac/WebInspector/WebInspector.mm
> diff --git a/WebKit/mac/WebInspector/WebNodeHighlightView.mm
b/WebKit/mac/WebInspector/WebNodeHighlightView.mm
> diff --git a/WebKit/mac/WebView/WebView.mm b/WebKit/mac/WebView/WebView.mm
> diff --git a/WebKit/mac/WebView/WebViewData.h
b/WebKit/mac/WebView/WebViewData.h
> diff --git a/WebKit/mac/WebView/WebViewData.mm
b/WebKit/mac/WebView/WebViewData.mm
> diff --git a/WebKit/mac/WebView/WebViewPrivate.h
b/WebKit/mac/WebView/WebViewPrivate.h
As we discussed offline, please leave these changes out. It doesn't make sense
to disable context menu support for code in WebKit/mac. When WebKit code for
iPhone lands, it will be refactored or moved instead of shared within
WebKit/mac. (Note that mac-specific files in WebCore are needed to prevent
excessive use of EXCLUDED_SOURCE_FILE_NAMES in Xcode.)
r=me
More information about the webkit-reviews
mailing list