Bug 137636 – Web Inspector: move unchecked InspectorValues into Protocol namespace

• Component: Web Inspector
• Status: RESOLVED INVALID

[Brian Burg]

In efforts to use unchecked inspector objects less, these are moved into the Protocol namespace. Subsequent patches will start converting agents to use Protocol::Domain::Object instances instead of unchecked objects.

Renamings:

Inspector{Object, Value, Array} -> Protocol::Unchecked{Object, Value, Array}
InspectorString, InspectorBasicValue -> Protocol::ScalarValue

Refactor:
* Remove ArrayItemHelper
* Combine InspectorValues.h and InspectorProtocolTypes.h

[Radar WebKit Bug Importer]

<rdar://problem/18623109>

[Brian Burg]

Created attachment 239678 [details]
Patch

[WebKit Commit Bot]

This patch modifies the WEB_REPLAY inputs generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-input-generator-tests --reset-results`)

This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)

[WebKit Commit Bot]

Attachment 239678 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:48:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:49:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:50:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:51:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:52:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:53:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:54:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:55:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:56:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:57:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:58:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:59:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:101:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:408:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:446:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:449:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:492:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:761:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:772:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.h:97:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.h:445:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.h:450:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.h:455:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.h:460:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.h:465:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.h:470:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.h:475:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.h:480:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.h:485:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/inspector/InspectorStyleSheet.h:66:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generator.py:47:  whitespace before '}'  [pep8/E202] [5]
ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generator_templates.py:253:  whitespace before ')'  [pep8/E202] [5]
Total errors found: 32 in 92 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brian Burg]

Looks like I will need to revert the InspectorString -> ScalarValue change. It's not safe to put WTF::String inside a union.

[Brian Burg]

Created attachment 239688 [details]
Patch

[WebKit Commit Bot]

Attachment 239688 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:48:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:49:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:50:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:51:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:52:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:53:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:54:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:55:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:56:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:57:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:58:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/InspectorProtocolTypes.cpp:59:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generator.py:47:  whitespace before '}'  [pep8/E202] [5]
ERROR: Source/JavaScriptCore/inspector/scripts/codegen/generator_templates.py:253:  whitespace before ')'  [pep8/E202] [5]
Total errors found: 14 in 92 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Joseph Pecoraro]

Comment on attachment 239688 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=239688&action=review

Looks good to me. Despite my initial aversion to the name it actually is reasonable and a keen naming choice.

> Source/JavaScriptCore/ChangeLog:11
> +        whose fields are shape-checked when going across the channel to or from the frontend>

Typo: '>'

> Source/JavaScriptCore/inspector/ConsoleMessage.cpp:227
> -        jsonObj->setStackTrace(m_callStack->buildInspectorArray());
> +        jsonObj->setStackTrace(m_callStack->buildUncheckedArray());

Maybe it is just me, but I find "buildInspectorArray" to be clearer than "buildUncheckedArray".

> Source/WebCore/inspector/InspectorApplicationCacheAgent.h:39
> +class UncheckedObject;
> +class UncheckedValue;

In the wrong namespace. Needs namespace Protocol {}

> Source/WebCore/inspector/InspectorPageAgent.h:47
> +class UncheckedArray;

Namespace.

> Source/WebCore/inspector/InspectorResourceAgent.h:45
> +class UncheckedArray;

Namespace.

> Source/WebCore/inspector/InspectorWorkerAgent.h:40
> +class UncheckedObject;

Namespace. Given that these are in the wrong namespace maybe they are unnecessary forward declarations due to the includes. So maybe they can just be removed.

[Joseph Pecoraro]

I don't think this is applicable anymore, right?