Bug 82958

Summary: Web Inspector: CodeGeneratorInspector.py: switch Inspector, Memory and Database domains to typed API
Product: WebKit Reporter: Peter Rybin <prybin>
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 72861    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
yurys: review+
Patch for landing (with style errors fixed) none

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>