Bug 29260 – Add ENABLE(INSPECTOR)

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Greg Bolsinga]

Wrap Inspector code in an ENABLE wrapper. This is for platforms (such as
iPhone) that do not support the Inspector.

<rdar://problem/6732605>

[Greg Bolsinga]

Created attachment 39577 [details]
This patch implements the needed changes

This will compile and link Mac OS X without Inspector support.

Of course Mac OS X (and many other platforms) will never ship in this
configuration, so should the Mac code even be conditionally compiled for this?

[Greg Bolsinga]

This is being conditionalized because it is turned off on iPhone.

[David Kilzer (:ddkilzer)]

Comment on attachment 39577 [details]
This patch implements the needed changes

> 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

[Greg Bolsinga]

(In reply to comment #3)
> > --- 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).

I got the rest, but this can't be done since ContextMenuItemTagInspectElement does not exist unless ENABLE(INSPECTOR) is true.

[Greg Bolsinga]

http://trac.webkit.org/changeset/48430