154509 – Web Inspector: add 'Automation' protocol domain and generate its backend classes separately in WebKit2

RESOLVED FIXED 154509

Web Inspector: add 'Automation' protocol domain and generate its backend classes separately in WebKit2

https://bugs.webkit.org/show_bug.cgi?id=154509

Summary Web Inspector: add 'Automation' protocol domain and generate its backend clas...

Blaze Burg

Reported 2016-02-20 22:53:04 PST

Yup.

Attachments
Proposed Fix (46.40 KB, patch) 2016-02-20 23:27 PST, Blaze Burg	no flags	Details Formatted Diff Diff
Test the CMake waters (46.25 KB, patch) 2016-02-21 16:12 PST, Blaze Burg	no flags	Details Formatted Diff Diff
CMake try #2 (46.90 KB, patch) 2016-02-21 16:19 PST, Blaze Burg	no flags	Details Formatted Diff Diff
Take 2 (47.77 KB, patch) 2016-02-22 13:16 PST, Blaze Burg	no flags	Details Formatted Diff Diff
For EWS, Landing (47.70 KB, patch) 2016-02-22 18:45 PST, Blaze Burg	no flags	Details Formatted Diff Diff
For Landing (47.71 KB, patch) 2016-02-22 22:18 PST, Blaze Burg	no flags	Details Formatted Diff Diff
Simple test producing crashes on GTK+ Debug (377 bytes, text/html) 2016-02-26 06:21 PST, Adrien Plazas	no flags	Details
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2016-02-20 22:53:20 PST

<rdar://problem/24759098>

Blaze Burg

Comment 2 2016-02-20 23:27:02 PST

Created attachment 271877 [details] Proposed Fix

Timothy Hatcher

Comment 3 2016-02-21 14:44:32 PST

Comment on attachment 271877 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=271877&action=review > Source/WebKit2/UIProcess/Automation/Automation.json:33 > + "description": "Closes the specified window." Copy paste error: Opens a window.

Timothy Hatcher

Comment 4 2016-02-21 14:46:25 PST

Comment on attachment 271877 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=271877&action=review >> Source/WebKit2/UIProcess/Automation/Automation.json:33 >> + "description": "Closes the specified window." > > Copy paste error: Opens a window. I see you fixed this in the patch for bug 154518.

Blaze Burg

Comment 5 2016-02-21 15:00:08 PST

(In reply to comment #4) > Comment on attachment 271877 [details] > Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271877&action=review > > >> Source/WebKit2/UIProcess/Automation/Automation.json:33 > >> + "description": "Closes the specified window." > > > > Copy paste error: Opens a window. > > I see you fixed this in the patch for bug 154518. I'll move the changes to this patch before landing.

Blaze Burg

Comment 6 2016-02-21 16:12:51 PST

Created attachment 271890 [details] Test the CMake waters

WebKit Commit Bot

Comment 7 2016-02-21 16:15:16 PST

Attachment 271890 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:230: multiple statements on one line (semicolon) [pep8/E702] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:231: multiple statements on one line (semicolon) [pep8/E702] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:165: [generate_from_specification] Undefined variable 'CppBackendDispatcherHeaderGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:166: [generate_from_specification] Undefined variable 'CppBackendDispatcherImplementationGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:167: [generate_from_specification] Undefined variable 'CppProtocolTypesHeaderGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:168: [generate_from_specification] Undefined variable 'CppProtocolTypesImplementationGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:172: [generate_from_specification] Undefined variable 'ObjCConversionHelpersGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:173: [generate_from_specification] Undefined variable 'ObjCFrontendDispatcherImplementationGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:174: [generate_from_specification] Undefined variable 'ObjCProtocolTypesImplementationGenerator' [pylint/E0602] [5] Total errors found: 9 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.

Blaze Burg

Comment 8 2016-02-21 16:19:59 PST

Created attachment 271891 [details] CMake try #2

WebKit Commit Bot

Comment 9 2016-02-21 16:21:56 PST

Attachment 271891 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:230: multiple statements on one line (semicolon) [pep8/E702] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:231: multiple statements on one line (semicolon) [pep8/E702] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:165: [generate_from_specification] Undefined variable 'CppBackendDispatcherHeaderGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:166: [generate_from_specification] Undefined variable 'CppBackendDispatcherImplementationGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:167: [generate_from_specification] Undefined variable 'CppProtocolTypesHeaderGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:168: [generate_from_specification] Undefined variable 'CppProtocolTypesImplementationGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:172: [generate_from_specification] Undefined variable 'ObjCConversionHelpersGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:173: [generate_from_specification] Undefined variable 'ObjCFrontendDispatcherImplementationGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:174: [generate_from_specification] Undefined variable 'ObjCProtocolTypesImplementationGenerator' [pylint/E0602] [5] Total errors found: 9 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Blaze Burg

Comment 10 2016-02-21 20:49:26 PST

Committed r196891: <http://trac.webkit.org/changeset/196891>

Joseph Pecoraro

Comment 11 2016-02-21 23:25:49 PST

Comment on attachment 271891 [details] CMake try #2 View in context: https://bugs.webkit.org/attachment.cgi?id=271891&action=review > Source/WebKit2/DerivedSources.make:214 > +JSON_RPC_OUTPUT_FILES = \ JSON_RPC seems vague. So is outputting files named InspectorBackendDispatcher. Could we s/Inspector/Automation/ eventually? > Source/WebKit2/UIProcess/Automation/Automation.json:26 > + "description": "Gets information about all open windows and tabs in the automation session." Perhaps we could take this opportunity to put the "description" next to the "name" in the new JSON files? This order always bugged me! > Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp:31 > +#include <JavaScriptCore/InspectorBackendDispatcher.h> > +#include <JavaScriptCore/InspectorFrontendRouter.h> Hmm, I had thought this was an issue with other ports in the past. But maybe they don't build these files, in which case it seems fine to use this <JavaScriptCore/...> syntax.

Blaze Burg

Comment 12 2016-02-22 08:44:48 PST

(In reply to comment #11) > Comment on attachment 271891 [details] > CMake try #2 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271891&action=review > > > Source/WebKit2/DerivedSources.make:214 > > +JSON_RPC_OUTPUT_FILES = \ > > JSON_RPC seems vague. So is outputting files named > InspectorBackendDispatcher. Could we s/Inspector/Automation/ eventually? Wasn't sure of a better prefix than JSON_RPC. Maybe add 'Protocol' in for good measure. At least it doesn't say Inspector. I plan to change the Inspector* filename prefix in the future for generated protocol files. Just wanted to get it working for now. > > Source/WebKit2/UIProcess/Automation/Automation.json:26 > > + "description": "Gets information about all open windows and tabs in the automation session." > > Perhaps we could take this opportunity to put the "description" next to the > "name" in the new JSON files? This order always bugged me! OK > > Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp:31 > > +#include <JavaScriptCore/InspectorBackendDispatcher.h> > > +#include <JavaScriptCore/InspectorFrontendRouter.h> > > Hmm, I had thought this was an issue with other ports in the past. But maybe > they don't build these files, in which case it seems fine to use this > <JavaScriptCore/...> syntax. These includes will pull from ForwardingHeaders or headers installed to ${BUILT_PRODUCTS_DIR}/usr/lib/include. Every port does this, far as I know.

WebKit Commit Bot

Comment 13 2016-02-22 08:59:30 PST

Re-opened since this is blocked by bug 154539

Blaze Burg

Comment 14 2016-02-22 13:16:11 PST

Created attachment 271948 [details] Take 2

Blaze Burg

Comment 15 2016-02-22 18:45:32 PST

Created attachment 271980 [details] For EWS, Landing

WebKit Commit Bot

Comment 16 2016-02-22 18:47:15 PST

Attachment 271980 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:230: multiple statements on one line (semicolon) [pep8/E702] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:231: multiple statements on one line (semicolon) [pep8/E702] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:165: [generate_from_specification] Undefined variable 'CppBackendDispatcherHeaderGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:166: [generate_from_specification] Undefined variable 'CppBackendDispatcherImplementationGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:167: [generate_from_specification] Undefined variable 'CppProtocolTypesHeaderGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:168: [generate_from_specification] Undefined variable 'CppProtocolTypesImplementationGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:172: [generate_from_specification] Undefined variable 'ObjCConversionHelpersGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:173: [generate_from_specification] Undefined variable 'ObjCFrontendDispatcherImplementationGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:174: [generate_from_specification] Undefined variable 'ObjCProtocolTypesImplementationGenerator' [pylint/E0602] [5] Total errors found: 9 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Blaze Burg

Comment 17 2016-02-22 22:18:26 PST

Created attachment 271989 [details] For Landing

WebKit Commit Bot

Comment 18 2016-02-22 22:22:59 PST

Attachment 271989 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:230: multiple statements on one line (semicolon) [pep8/E702] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:231: multiple statements on one line (semicolon) [pep8/E702] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:165: [generate_from_specification] Undefined variable 'CppBackendDispatcherHeaderGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:166: [generate_from_specification] Undefined variable 'CppBackendDispatcherImplementationGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:167: [generate_from_specification] Undefined variable 'CppProtocolTypesHeaderGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:168: [generate_from_specification] Undefined variable 'CppProtocolTypesImplementationGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:172: [generate_from_specification] Undefined variable 'ObjCConversionHelpersGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:173: [generate_from_specification] Undefined variable 'ObjCFrontendDispatcherImplementationGenerator' [pylint/E0602] [5] ERROR: Source/JavaScriptCore/inspector/scripts/generate-inspector-protocol-bindings.py:174: [generate_from_specification] Undefined variable 'ObjCProtocolTypesImplementationGenerator' [pylint/E0602] [5] Total errors found: 9 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

WebKit Commit Bot

Comment 19 2016-02-22 23:18:37 PST

Comment on attachment 271989 [details] For Landing Clearing flags on attachment: 271989 Committed r196970: <http://trac.webkit.org/changeset/196970>

WebKit Commit Bot

Comment 20 2016-02-22 23:18:42 PST

All reviewed patches have been landed. Closing bug.

Csaba Osztrogonác

Comment 21 2016-02-23 09:35:55 PST

(In reply to comment #19) > Comment on attachment 271989 [details] > For Landing > > Clearing flags on attachment: 271989 > > Committed r196970: <http://trac.webkit.org/changeset/196970> It made fast/profiler tests crash on EFL/GTK buildbots and made them exit early after 50+ crashes. See build.webkit.org for details. cc-ing port maintainers

Carlos Garcia Campos

Comment 22 2016-02-23 09:37:49 PST

This patch broke the inspector in GTK+ and EFL ports and caused a lot of crashes in both ports bots.

Blaze Burg

Comment 23 2016-02-23 11:57:36 PST

(In reply to comment #21) > (In reply to comment #19) > > Comment on attachment 271989 [details] > > For Landing > > > > Clearing flags on attachment: 271989 > > > > Committed r196970: <http://trac.webkit.org/changeset/196970> > > It made fast/profiler tests crash on EFL/GTK buildbots and made > them exit early after 50+ crashes. See build.webkit.org for details. > > cc-ing port maintainers This patch didn't touch anything related to the profiler, nor do these failures show on Mac/Win. So this is very surprising. There have been recent profiler changes, however.

Blaze Burg

Comment 24 2016-02-23 11:58:28 PST

(In reply to comment #22) > This patch broke the inspector in GTK+ and EFL ports and caused a lot of > crashes in both ports bots. You'll need to provide more information (hang? crash? nothing shows?) otherwise I can't really do anything about it.

Carlos Garcia Campos

Comment 25 2016-02-23 22:30:29 PST

(In reply to comment #24) > (In reply to comment #22) > > This patch broke the inspector in GTK+ and EFL ports and caused a lot of > > crashes in both ports bots. > > You'll need to provide more information (hang? crash? nothing shows?) > otherwise I can't really do anything about it. Yes, I didn't have time yesterday to debug the issue. The inspector made the web process crash, and I just confirmed that reverting r196970 fixed the issue. Now looking at the GTK+ bots it seems that r196980 fixed the crashes.

Carlos Alberto Lopez Perez

Comment 26 2016-02-24 09:57:20 PST

(In reply to comment #25) > (In reply to comment #24) > > (In reply to comment #22) > > > This patch broke the inspector in GTK+ and EFL ports and caused a lot of > > > crashes in both ports bots. > > > > You'll need to provide more information (hang? crash? nothing shows?) > > otherwise I can't really do anything about it. > > Yes, I didn't have time yesterday to debug the issue. The inspector made the > web process crash, and I just confirmed that reverting r196970 fixed the > issue. Now looking at the GTK+ bots it seems that r196980 fixed the crashes. r196980 fixed the issue in the release build, but it also caused the debug build to assert on all fast/profiler tests with this: fast/profiler/anonymous-function-called-from-different-contexts.html crashed, (stderr lines): ASSERTION FAILED: result == "EventDispatch" || result == "ScheduleStyleRecalculation" || result == "RecalculateStyles" || result == "InvalidateLayout" || result == "Layout" || result == "Paint" || result == "Composite" || result == "RenderingFrame" || result == "TimerInstall" || result == "TimerRemove" || result == "TimerFire" || result == "EvaluateScript" || result == "TimeStamp" || result == "Time" || result == "TimeEnd" || result == "FunctionCall" || result == "ProbeSample" || result == "ConsoleProfile" || result == "RequestAnimationFrame" || result == "CancelAnimationFrame" || result == "FireAnimationFrame" DerivedSources/JavaScriptCore/inspector/InspectorProtocolObjects.cpp(872) : static void Inspector::Protocol::BindingTraits<Inspector::Protocol::Timeline::EventType>::assertValueHasExpectedType(Inspector::InspectorValue*) 1 0x7fac2dbd77ef /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(WTFCrash+0x1e) [0x7fac2dbd77ef] 2 0x7fac2dbd3719 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN9Inspector8Protocol13BindingTraitsINS0_8Timeline9EventTypeEE26assertValueHasExpectedTypeEPNS_14InspectorValueE+0x331) [0x7fac2dbd3719] 3 0x7fac2dbd38fd /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN9Inspector8Protocol13BindingTraitsINS0_8Timeline13TimelineEventEE26assertValueHasExpectedTypeEPNS_14InspectorValueE+0x1b5) [0x7fac2dbd38fd] 4 0x7fac2dbd3c67 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN9Inspector8Protocol13BindingTraitsINS0_8Timeline13TimelineEventEE11runtimeCastEON3WTF6RefPtrINS_14InspectorValueEEE+0x85) [0x7fac2dbd3c67] 5 0x7fac34dfc77c /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore22InspectorTimelineAgent19addRecordToTimelineEON3WTF6RefPtrIN9Inspector15InspectorObjectEEENS_18TimelineRecordTypeE+0x108) [0x7fac34dfc77c] 6 0x7fac34dfcbcc /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore22InspectorTimelineAgent22didCompleteRecordEntryERKNS0_19TimelineRecordEntryE+0x1d0) [0x7fac34dfcbcc] 7 0x7fac34dfb44c /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore22InspectorTimelineAgent15stopFromConsoleEPN3JSC9ExecStateERKN3WTF6StringE+0x18c) [0x7fac34dfb44c] 8 0x7fac34da9309 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore24InspectorInstrumentation17stopProfilingImplERNS_19InstrumentingAgentsEPN3JSC9ExecStateERKN3WTF6StringE+0x47) [0x7fac34da9309] 9 0x7fac34fc405b /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore24InspectorInstrumentation13stopProfilingERNS_4PageEPN3JSC9ExecStateERKN3WTF6StringE+0x3b) [0x7fac34fc405b] 10 0x7fac34fc3bc5 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore17PageConsoleClient10profileEndEPN3JSC9ExecStateERKN3WTF6StringE+0x31) [0x7fac34fc3bc5] 11 0x7fac2d9719ec /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x1d589ec) [0x7fac2d9719ec] 12 0x7fabdd7fe0c8 [0x7fabdd7fe0c8] 0033/39138] fast/profiler/anonymous-function-called-from-different-contexts.html failed unexpectedly (WebProcess crashed)

Adrien Plazas

Comment 27 2016-02-26 06:21:39 PST

Created attachment 272322 [details] Simple test producing crashes on GTK+ Debug There are two lines in this test which, when any of the two is removed, make the test not crash.

Timothy Hatcher

Comment 28 2016-02-26 09:13:53 PST

A follow up bug would be more appropriate to track the assert you are seeing.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware All

OS All

Product WebKit

Component Web Inspector

Assignee

Blaze Burg

Reported

2016-02-20 22:53 PST

Modified

2016-02-26 09:13 PST History

CC List

13 users Show

URL

Keywords InRadar

Depends on

154539

Blocks

154518

Dependencies

tree graph