Bug 82958 - Web Inspector: CodeGeneratorInspector.py: switch Inspector, Memory and Database domains to typed API
Summary: Web Inspector: CodeGeneratorInspector.py: switch Inspector, Memory and Databa...
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: 72861
  Show dependency treegraph
 
Reported: 2012-04-02 15:38 PDT by Peter Rybin
Modified: 2012-04-04 02:36 PDT (History)
10 users (show)

See Also:


Attachments
Patch (13.52 KB, patch)
2012-04-02 15:44 PDT, Peter Rybin
no flags Details | Formatted Diff | Diff
Patch (13.36 KB, patch)
2012-04-03 08:36 PDT, Peter Rybin
no flags Details | Formatted Diff | Diff
Patch (13.35 KB, patch)
2012-04-03 09:03 PDT, Peter Rybin
yurys: review+
Details | Formatted Diff | Diff
Patch for landing (with style errors fixed) (12.73 KB, patch)
2012-04-04 02:34 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Rybin 2012-04-02 15:38:15 PDT
Do not allow InspectorObject and other raw interfaces in Inspector, Memory and Database domains anymore.
Comment 1 Peter Rybin 2012-04-02 15:44:38 PDT
Created attachment 135208 [details]
Patch
Comment 2 Peter Rybin 2012-04-02 15:46:41 PDT
Please, take a look at new FIXME. This probably should be resovled before commit.
Comment 3 WebKit Review Bot 2012-04-02 15:48:27 PDT
Attachment 135208 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/inspector/InspectorDatabaseAgent.cpp:116:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Yury Semikhatsky 2012-04-02 22:00:59 PDT
Comment on attachment 135208 [details]
Patch

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

> Source/WebCore/inspector/InspectorDatabaseResource.cpp:62
> +        // FIXME: here we use string type according to Inspector.json.

Please file a bug on this and put its number next to FIXME.

> Source/WebCore/inspector/InspectorMemoryAgent.cpp:205
> +    PassRefPtr<TypeBuilder::Array<TypeBuilder::Memory::DOMGroup> > domGroups() { return m_domGroups; }

Why did it become PassRefPtr?
Comment 5 Yury Semikhatsky 2012-04-02 23:40:18 PDT
(In reply to comment #4)
> (From update of attachment 135208 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=135208&action=review
> 
> > Source/WebCore/inspector/InspectorDatabaseResource.cpp:62
> > +        // FIXME: here we use string type according to Inspector.json.
> 
> Please file a bug on this and put its number next to FIXME.
> 
Nevermind, I've sent a fix for this https://bugs.webkit.org/show_bug.cgi?id=83003
Comment 6 Peter Rybin 2012-04-03 08:36:48 PDT
Created attachment 135334 [details]
Patch
Comment 7 Peter Rybin 2012-04-03 08:37:10 PDT
Comment on attachment 135208 [details]
Patch

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

>> Source/WebCore/inspector/InspectorMemoryAgent.cpp:205
>> +    PassRefPtr<TypeBuilder::Array<TypeBuilder::Memory::DOMGroup> > domGroups() { return m_domGroups; }
> 
> Why did it become PassRefPtr?

Because we work in terms of RefPtr's everywhere here and there is no reason for transient switch to raw pointer.
A drive-by fix.
Comment 8 WebKit Review Bot 2012-04-03 08:40:09 PDT
Attachment 135334 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/inspector/InspectorDatabaseAgent.cpp:116:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Peter Rybin 2012-04-03 09:03:00 PDT
Created attachment 135339 [details]
Patch
Comment 10 WebKit Review Bot 2012-04-03 09:06:39 PDT
Attachment 135339 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/inspector/InspectorDatabaseAgent.cpp:116:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Yury Semikhatsky 2012-04-04 02:34:13 PDT
Created attachment 135535 [details]
Patch for landing (with style errors fixed)
Comment 12 Yury Semikhatsky 2012-04-04 02:35:30 PDT
(In reply to comment #10)
> Attachment 135339 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
> Source/WebCore/inspector/InspectorDatabaseAgent.cpp:116:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
> Total errors found: 1 in 10 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

Peter, next time please make sure style errors like this are resolved before landing.
Comment 13 Yury Semikhatsky 2012-04-04 02:36:48 PDT
Committed r113157: <http://trac.webkit.org/changeset/113157>