Bug 56681 - Web Inspector: expose object id as string, not JSON struct in the protocol.
Summary: Web Inspector: expose object id as string, not JSON struct in the protocol.
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: 2011-03-18 14:12 PDT by Pavel Feldman
Modified: 2011-03-21 00:42 PDT (History)
10 users (show)

See Also:

Attachments
Patch (42.98 KB, patch)
2011-03-19 02:44 PDT, Pavel Feldman 		yurys: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2011-03-18 14:12:16 PDT 
I'm brushing up the protocol and it seems like exposing ObjectId structure is not really necessary. I'm actually thinking of making all ids (network resources, dom nodes, javascript objects, call frames and page frames) simple strings.
Comment 1 Pavel Feldman 2011-03-19 02:44:23 PDT 
Created attachment 86257 [details]
Patch
Comment 2 Ilya Tikhonovsky 2011-03-19 02:49:44 PDT 
Comment on attachment 86257 [details]
Patch

lgtm
Comment 3 Yury Semikhatsky 2011-03-21 00:16:37 PDT 
Comment on attachment 86257 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=86257&action=review

> LayoutTests/http/tests/inspector/protocol-test.js:5
> +    if (object instanceof Object)

Consider renaming object to value. object sounds like an instance of Object while the parameter can take integer values.

> Source/WebCore/inspector/InjectedScriptManager.cpp:83
> +        bool success = parsedObjectId->asObject()->getNumber("injectedScriptId", &injectedScriptId);

Now that objectId is an opaque string for the front-end we can switch to a more compact format, probably two numbers separated by a column.
Comment 4 Yury Semikhatsky 2011-03-21 00:16:59 PDT 
Comment on attachment 86257 [details]
Patch

Please address the comments before landing.
Comment 5 Pavel Feldman 2011-03-21 00:42:57 PDT 
Committed r81570: <http://trac.webkit.org/changeset/81570>