Bug 54856 - Web Inspector: Web Inspector protocol metabug.
Summary: Web Inspector: Web Inspector protocol metabug.
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 54958 56806 56815
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-20 23:43 PST by Pavel Feldman
Modified: 2014-01-25 14:54 PST (History)
12 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2011-02-20 23:43:19 PST
Each entry here should be fixed as a bug. I won't be creating bugs right now because there are too many:

Raw message format:
For following items see description in https://docs.google.com/a/google.com/document/edit?id=1d_N-OIb3UztuC-_g0piXsIIdp89HnMcDk-uSQ8uamW4&hl=en&authkey=CIf48O4J&pli=1#
- Each event and command request should have "seq" number, each command response should have "seq" and "request_seq". Rationale: we should be able to tell that the message was lost.
- Each command response should have boolean success and string error message as in protocol above.
- All messages should explicitly state their type (request, response, event)
- Response data should live in attribute "body"
- Rename [notify] into [event] in the IDL
- We should never use Value in the protocol. Should be either Array or Object (or primitive number)
- Some parameters should be marked as optional with default values:
        Console::evaluate(in String expression, in String objectGroup, in [optional, default=0] number callFrameId, in [optional, default=true] boolean includeCommandLineAPI, out [typeRef=objectRef] Object result);
- All events should use notification wording, not command wording: addConsoleMessage->consoleMessage, updateConsoleMessageRepeatCount->consoleMessageRepeatCountUpdates, etc.

Domain-specific changes:
[Inspector]
- Move setUserAgentOverride to the Network agent
- Move addNodesToSearchResult to DOMAgent. Abstract agent from performing UI search into executing asynchronous text search managed from the front-end.
- Move start/stopTimelineProfiler to the Timeline agent
- Same for debugging and profiling
- disconnectFromBackend should vanish. We should not ask front-end to initiate disconnect.

[Runtime]
- Merge Runtime agent into Console agent (all it really does is interacting with console)
- For all Runtime messages, out parameter should be more specific (Object, Array, number)
- Worker messages should be moved to worker agent

[InjectedScript]
- Agent should be removed

[DOM]
- Should not push nodes into front-end, should rather request nodes

[CSS]
- All out parameters are broken (Values)

[Timeline]
- Remove timelineProfilerWasStarted / Stopped in favor of start/stop callbacks. This has WebKit implications that need to be resolved.

[Debugger]
- Same for debuggerWasEnabled / debuggerWasDisabled
- activateBreakpoints/deactivateBreakpoints -> setBreakpointsActive(bool)
- We can merge setJavaScriptBreakpoint and setJavaScriptBreakpointBySourceId once protocol allows optional parameters
- We might want to merge evaluateOnCallFrame and getCompletionsOnCallFrame once optional parameters are allowed
- Move workers into separate agent

[BrowserDebugger]
- setAllBrowserBreakpoints should be further split

[Profiler]
- Needs major review
Comment 1 Ilya Tikhonovsky 2011-02-22 07:39:53 PST
Done: - Each event and command request should have "seq" number, each command response should have "seq" and "request_seq". Rationale: we should be able to tell that the message was lost.
Comment 2 Ilya Tikhonovsky 2011-02-24 04:11:45 PST
landed r79539, r79542 - Each command response should have boolean success and string error message as in protocol above.
Comment 3 Ilya Tikhonovsky 2011-02-24 06:54:13 PST
r79558 - Response data should live in attribute "body"
Comment 4 Ilya Tikhonovsky 2011-02-24 06:55:25 PST
r79325 - Rename [notify] into [event] in the IDL
Comment 5 Ilya Tikhonovsky 2011-03-11 06:00:16 PST
r80845 - Each command response should have boolean success and string error message as in protocol above.

it was decided that we don't want to have separate success flag.
it can be emulated as !response.error.
Comment 6 Ilya Tikhonovsky 2011-03-11 06:03:05 PST
r80485 - Each event and command request should have "seq" number, each command response should have "seq" and "request_seq". Rationale: we should be able to tell that the message was lost.

it was decided to:
1) use id instead of seq for the requests;
2) requestId instead of request_seq for the responses;
3) drop seq for the responses.