Bug 26010 - Serialize calls to WebInspector
Summary: Serialize calls to WebInspector
Status: RESOLVED FIXED
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:
Blocks:
 
Reported: 2009-05-25 09:05 PDT by Pavel Feldman
Modified: 2009-05-26 05:24 PDT (History)
2 users (show)

See Also:


Attachments
patch (16.29 KB, patch)
2009-05-25 09:09 PDT, Pavel Feldman
timothy: review-
Details | Formatted Diff | Diff
patch (16.46 KB, patch)
2009-05-25 13:32 PDT, Pavel Feldman
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2009-05-25 09:05:20 PDT
Serialize calls to WebInspector. This is the first step where method calls are being serialized to arrays (not yet JSON stringified ones) and are being dispatched on the client. This change also allows client to override InspectorFrontend, so that the serialized calls can be made on a given object instead of in-process WebInspector global. This will be the main control flow when InspectorController is decoupled from the in-process frontend.
Comment 1 Pavel Feldman 2009-05-25 09:09:30 PDT
Created attachment 30652 [details]
patch
Comment 2 Timothy Hatcher 2009-05-25 10:55:30 PDT
Comment on attachment 30652 [details]
patch

> +    overrideFrontendObject(m_scriptState, webInspectorObj);

I think "proxy" is a better term for this than "override".

Maybe frontendProxyObject?

> +WebInspector.dispatch = function() {
> +  var methodName = arguments[0];
> +  var parameters = Array.prototype.slice.call(arguments, 1);
> +  WebInspector[methodName].apply(this, parameters);
> +}

Please use 4 spaces for indenting.

r- for that. Otherwise r+.
Comment 3 Mark Rowe (bdash) 2009-05-25 12:41:25 PDT
Comment on attachment 30652 [details]
patch

> +ScriptFunctionCall* InspectorFrontend::newFunctionCall(const String& functionName)
> +{
> +    ScriptFunctionCall* function = new ScriptFunctionCall(m_scriptState, m_webInspector, "dispatch");
> +    function->appendArgument(functionName);
> +    return function;
> +}

This would be an ideal place to use PassOwnPtr rather than a raw pointer, to indicate that ownership of an object is being transferred out of the function.
Comment 4 Pavel Feldman 2009-05-25 13:32:23 PDT
Created attachment 30659 [details]
patch

(In reply to comment #2)
> (From update of attachment 30652 [details] [review])
> > +    overrideFrontendObject(m_scriptState, webInspectorObj);
> 
> I think "proxy" is a better term for this than "override".
> 
> Maybe frontendProxyObject?
> 

Renamed to setFrontendProxyObject

> > +WebInspector.dispatch = function() {
> > +  var methodName = arguments[0];
> > +  var parameters = Array.prototype.slice.call(arguments, 1);
> > +  WebInspector[methodName].apply(this, parameters);
> > +}
> 
> Please use 4 spaces for indenting.
> 

Done. Sorry about that - old habit.

> r- for that. Otherwise r+.
> 

(In reply to comment #3)
> (From update of attachment 30652 [details] [review])
> > +ScriptFunctionCall* InspectorFrontend::newFunctionCall(const String& functionName)
> > +{
> > +    ScriptFunctionCall* function = new ScriptFunctionCall(m_scriptState, m_webInspector, "dispatch");
> > +    function->appendArgument(functionName);
> > +    return function;
> > +}
> 
> This would be an ideal place to use PassOwnPtr rather than a raw pointer, to
> indicate that ownership of an object is being transferred out of the function.
> 

Done.
Comment 5 Alexey Proskuryakov 2009-05-26 05:24:47 PDT
Committed revision 44149.