Bug 137636

Summary:

Web Inspector: move unchecked InspectorValues into Protocol namespace

Product:

WebKit

Reporter:

Brian Burg <burg>

Component:

Web Inspector

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED INVALID

Severity:

Normal

CC:

bburg, commit-queue, dbates, esprehn+autocc, graouts, gyuyoung.kim, inspector-bugzilla-changes, joepeck, mkwst, rakuco, ryuan.choi, sergio, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	joepeck: review+

Description Brian Burg 2014-10-11 10:00:00 PDT

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

Comment 1 Radar WebKit Bug Importer 2014-10-11 10:00:37 PDT

<rdar://problem/18623109>

Comment 2 Brian Burg 2014-10-11 10:16:13 PDT

Created attachment 239678 [details]
Patch

Comment 3 WebKit Commit Bot 2014-10-11 10:16:59 PDT

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`)

Comment 4 WebKit Commit Bot 2014-10-11 10:17:25 PDT

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.

Comment 5 Brian Burg 2014-10-11 11:33:22 PDT

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

Comment 6 Brian Burg 2014-10-11 15:29:08 PDT

Created attachment 239688 [details]
Patch

Comment 7 WebKit Commit Bot 2014-10-11 15:30:46 PDT

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.

Comment 8 Joseph Pecoraro 2014-10-16 12:09:37 PDT

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.

Comment 9 Joseph Pecoraro 2018-01-24 14:42:51 PST

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