Bug 40064 - Web Inspector: Implement JSON parsing for InspectorValue.
Summary: Web Inspector: Implement JSON parsing for InspectorValue.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-02 08:22 PDT by Pavel Podivilov
Modified: 2010-06-07 04:33 PDT (History)
3 users (show)

See Also:


Attachments
Proposed patch. (17.43 KB, patch)
2010-06-02 08:37 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Proposed patch. (16.46 KB, patch)
2010-06-02 09:02 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Proposed patch. (19.18 KB, patch)
2010-06-03 06:31 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Proposed patch. (19.59 KB, patch)
2010-06-03 08:02 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Podivilov 2010-06-02 08:22:01 PDT
Web Inspector: Implement JSON parsing for InspectorValue.
Comment 1 Pavel Podivilov 2010-06-02 08:37:17 PDT
Created attachment 57658 [details]
Proposed patch.
Comment 2 Pavel Podivilov 2010-06-02 09:02:46 PDT
Created attachment 57661 [details]
Proposed patch.
Comment 3 Pavel Podivilov 2010-06-03 06:31:42 PDT
Created attachment 57761 [details]
Proposed patch.
Comment 4 Pavel Feldman 2010-06-03 06:55:51 PDT
Comment on attachment 57761 [details]
Proposed patch.

Function names should start with lower case.

WebCore/inspector/InspectorValues.cpp:40
 +  static const int kStackLimit = 100;
1000 ?

WebCore/inspector/InspectorValues.cpp:40
 +  static const int kStackLimit = 100;
no k prefix, start from capital.

WebCore/inspector/InspectorValues.cpp:57
 +  const char* const kNullString = "null";
no k prefix here and below.

WebCore/inspector/InspectorValues.cpp:63
 +      while (start < end && *token != '\0' && *start++ == *token++) {
{ } on same line.

Overall looks good. Please make the code conform to the WebKit style guidelines.
Comment 5 Pavel Podivilov 2010-06-03 08:02:44 PDT
Created attachment 57768 [details]
Proposed patch.
Comment 6 Pavel Feldman 2010-06-03 08:32:25 PDT
Comment on attachment 57768 [details]
Proposed patch.

This looks good to me. I think we need tests for this. We might need to wire this code to the actual inspector transport in order to use layout tests infrastructure...
Comment 7 WebKit Commit Bot 2010-06-07 04:33:25 PDT
Comment on attachment 57768 [details]
Proposed patch.

Clearing flags on attachment: 57768

Committed r60774: <http://trac.webkit.org/changeset/60774>
Comment 8 WebKit Commit Bot 2010-06-07 04:33:30 PDT
All reviewed patches have been landed.  Closing bug.