| Summary: | Web Inspector: inspector protocol should be switched from array to object based representation. | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ilya Tikhonovsky <loislo> | ||||||||||||||||||
| Component: | Web Inspector (Deprecated) | Assignee: | Ilya Tikhonovsky <loislo> | ||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||
| Severity: | Normal | CC: | bweinstein, dglazkov, eric, gustavo, joepeck, keishi, pfeldman, pmuellr, rik, timothy, webkit-ews, webkit.review.bot, xan.lopez, yurys | ||||||||||||||||||
| Priority: | P2 | ||||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||||
| Hardware: | All | ||||||||||||||||||||
| OS: | All | ||||||||||||||||||||
| Bug Depends on: | 44416 | ||||||||||||||||||||
| Bug Blocks: | |||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||
|
Description
Ilya Tikhonovsky
2010-08-20 09:02:30 PDT
Created attachment 64973 [details]
[patch] initial version.
Ilya, could you attach a generated version of InspectorBackendStub after you patch? Some minor comments after a glance. WebCore/inspector/CodeGeneratorInspector.pm:527 > + this[command] = this.sendMessageToBackend.bind(this, commandInfo); I think _registerDelegate should just take a single argument, commandInfo, and "command" is just "commandInfo.command". WebCore/inspector/CodeGeneratorInspector.pm:540 > + if (args.length == 1 && typeof args[0] === "function") Nit: === Created attachment 64978 [details]
sample: InspectorBackendDispatcher.cpp
Created attachment 64979 [details]
sample: InspectorBackendStub.js
Created attachment 64980 [details]
sample: RemoteInspectorFrontend.cpp
(In reply to comment #2) > Ilya, could you attach a generated version of InspectorBackendStub after you patch? > Some minor comments after a glance. done > WebCore/inspector/CodeGeneratorInspector.pm:527 > > + this[command] = this.sendMessageToBackend.bind(this, commandInfo); > I think _registerDelegate should just take a single argument, commandInfo, and "command" is just "commandInfo.command". > WebCore/inspector/CodeGeneratorInspector.pm:540 > > + if (args.length == 1 && typeof args[0] === "function") > Nit: === Attachment 64973 [details] did not build on win: Build output: http://queues.webkit.org/results/3742425 Comment on attachment 64973 [details]
[patch] initial version.
WebCore/inspector/CodeGeneratorInspector.pm:295
+ push(@function, " RefPtr<InspectorObject> requestMessageObject(requestMessageObjectRef);");
This is not needed.
WebCore/inspector/CodeGeneratorInspector.pm:511
+ "\\\"arguments\\\": {$argumentNames}" .
Unrelated to this issue, but it would be nice if we didn't escape quotes in the JSON strings in InspectorBackendStub.js but use single quotes instead( "\"property\"" -> '"property"')
WebCore/inspector/front-end/inspector.js:637
+ WebInspector.log("Error: InspectorBackend." + message.command + " failed.");
These messages should go into console.error not the Inspector's console.
WebCore/inspector/CodeGeneratorInspector.pm:354
+ push(@function, " responseMessage->setString(\"type\", \"response\");");
It might make sense to extract these string constants and reuse them.
Created attachment 65096 [details] [patch] second iteration > WebCore/inspector/CodeGeneratorInspector.pm:527 > > + this[command] = this.sendMessageToBackend.bind(this, commandInfo); > I think _registerDelegate should just take a single argument, commandInfo, and "command" is just "commandInfo.command". done. > WebCore/inspector/CodeGeneratorInspector.pm:540 > > + if (args.length == 1 && typeof args[0] === "function") > Nit: === done. > WebCore/inspector/CodeGeneratorInspector.pm:295 > + push(@function, " RefPtr<InspectorObject> requestMessageObject(requestMessageObjectRef);"); > This is not needed. done > WebCore/inspector/CodeGeneratorInspector.pm:511 > + "\\\"arguments\\\": {$argumentNames}" . > Unrelated to this issue, but it would be nice if we didn't escape quotes in the JSON strings in InspectorBackendStub.js but use single quotes instead( "\"property\"" -> '"property"') done > WebCore/inspector/front-end/inspector.js:637 > + WebInspector.log("Error: InspectorBackend." + message.command + " failed."); > These messages should go into console.error not the Inspector's console. done > WebCore/inspector/CodeGeneratorInspector.pm:354 > + push(@function, " responseMessage->setString(\"type\", \"response\");"); > It might make sense to extract these string constants and reuse them. done InspectorFrontendClientLocal::setAttachedWindow was fixed. WebInspector_syncDispatch was fixed. Created attachment 65097 [details]
sample: InspectorBackendStub.js
Attachment 65096 [details] did not build on mac: Build output: http://queues.webkit.org/results/3728546 Attachment 65096 [details] did not build on qt: Build output: http://queues.webkit.org/results/3789453 Attachment 65096 [details] did not build on chromium: Build output: http://queues.webkit.org/results/3775586 Created attachment 65103 [details]
[patch] second iteration. Rebaselined.
Comment on attachment 65103 [details]
[patch] second iteration. Rebaselined.
WebCore/inspector/front-end/inspector.js:638
+ console.log("Error: InspectorBackend." + messageObject.command + " failed.");
console.log -> console.error
Attachment 65096 [details] did not build on gtk: Build output: http://queues.webkit.org/results/3787410 Committed r65803 M WebCore/ChangeLog M WebCore/inspector/CodeGeneratorInspector.pm M WebCore/inspector/InspectorController.cpp M WebCore/inspector/InspectorFrontendClientLocal.cpp M WebCore/inspector/InspectorValues.h M WebCore/inspector/Inspector.idl M WebCore/inspector/front-end/inspector.js M WebCore/inspector/front-end/Callback.js M WebCore/inspector/InspectorController.h r65803 = f2ffcc11e3506172e312f7f20d457460ec35a38c (refs/remotes/trunk) With fixed windows build Committed r65809 M WebCore/ChangeLog M WebCore/inspector/CodeGeneratorInspector.pm M WebCore/inspector/InspectorController.cpp M WebCore/inspector/InspectorFrontendClientLocal.cpp M WebCore/inspector/InspectorValues.h M WebCore/inspector/Inspector.idl M WebCore/inspector/front-end/inspector.js M WebCore/inspector/front-end/Callback.js M WebCore/inspector/InspectorController.h r65809 = 12044a8f1a0b8a82f217ae39411f717045555612 (refs/remotes/trunk) Created attachment 65117 [details]
actually landed
|