Bug 78390 – Web Inspector: Switch Debugger agent to TypeBuilder

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Peter Rybin]

TypeBuilder provides type-safe API for output messages.
Switch one agent of domain "Debugger" to the new API.

[Peter Rybin]

Created attachment 126587 [details]
Patch

[WebKit Review Bot]

Attachment 126587 [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/InjectedScript.cpp:79:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.cpp:93:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.h:80:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.h:82:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 4 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Vsevolod Vlasov]

Comment on attachment 126587 [details]
Patch

I suggest adding "using" statements to implementation (e.g. like in InspectorIndexedDBAgent.cpp) to avoid flooding the code.

[Peter Rybin]

Created attachment 127029 [details]
Patch

[Early Warning System Bot]

Comment on attachment 127029 [details]
Patch

Attachment 127029 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11518411

[Gustavo Noronha (kov)]

Comment on attachment 127029 [details]
Patch

Attachment 127029 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11521386

[Gyuyoung Kim]

Comment on attachment 127029 [details]
Patch

Attachment 127029 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/11523375

[Peter Rybin]

Created attachment 127048 [details]
Patch

[Vsevolod Vlasov]

Comment on attachment 127048 [details]
Patch

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

> Source/WebCore/inspector/InspectorDebuggerAgent.cpp:324
> +    using WebCore::TypeBuilder::Debugger::Location;

Why isn't this statement at the beginning of the file?
This seems to be forbidden by style guide and you need to use a long name for return value type because of that.

[Peter Rybin]

(In reply to comment #9)
> (From update of attachment 127048 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=127048&action=review
> 
> > Source/WebCore/inspector/InspectorDebuggerAgent.cpp:324
> > +    using WebCore::TypeBuilder::Debugger::Location;
> 
> Why isn't this statement at the beginning of the file?
> This seems to be forbidden by style guide and you need to use a long name for return value type because of that.

Are you sure it IS forbidden?

Anyway Location clashes with WebCore::Location and it's why the previous patch didn't compile. I think this form might be optimal. Return value is OK, we have to do it in .h anyway.

[WebKit Review Bot]

Attachment 127029 [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/InjectedScript.cpp:85:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.cpp:99:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.h:80:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.h:82:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 4 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Attachment 127048 [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/InjectedScript.cpp:85:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.cpp:99:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.h:80:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.h:82:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 4 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Vsevolod Vlasov]

(In reply to comment #10)
> (In reply to comment #9)
> > (From update of attachment 127048 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=127048&action=review
> > 
> > > Source/WebCore/inspector/InspectorDebuggerAgent.cpp:324
> > > +    using WebCore::TypeBuilder::Debugger::Location;
> > 
> > Why isn't this statement at the beginning of the file?
> > This seems to be forbidden by style guide and you need to use a long name for return value type because of that.
> 
> Are you sure it IS forbidden?
> 
> Anyway Location clashes with WebCore::Location and it's why the previous patch didn't compile. I think this form might be optimal. Return value is OK, we have to do it in .h anyway.

5. In implementation files, put all other "using" statements at the beginning of the file, before any namespace definitions and after any "include" statements.

This sounds like it IS forbidden to me.
I am fine with using fully qualified name for Location.

[Peter Rybin]

Created attachment 127222 [details]
Patch

[Vsevolod Vlasov]

Comment on attachment 127222 [details]
Patch

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

> Source/WebCore/inspector/InspectorDebuggerAgent.h:133
> +    PassRefPtr<TypeBuilder::Array<TypeBuilder::Debugger::CallFrame > > currentCallFrames();

Extra space after CallFrame?

[Peter Rybin]

Created attachment 127231 [details]
Patch

[WebKit Review Bot]

Attachment 127222 [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/InjectedScript.cpp:85:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.cpp:99:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.h:80:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Source/WebCore/inspector/InjectedScript.h:82:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 4 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Attachment 127231 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
First, rewinding head to replay your work on top of it...
Applying: dataTransfer.types (HTML5 drag & drop) should return DOMStringList
Using index info to reconstruct a base tree...
<stdin>:84: trailing whitespace.
        
<stdin>:333: trailing whitespace.
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"
<stdin>:334: trailing whitespace.
 width="300" height="300" onload="runRepaintTest()">
<stdin>:335: trailing whitespace.
<script xlink:href="../../fast/repaint/resources/repaint.js" />
<stdin>:336: trailing whitespace.
<script><![CDATA[
warning: squelched 16 whitespace errors
warning: 21 lines add whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 dataTransfer.types (HTML5 drag & drop) should return DOMStringList

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Vsevolod Vlasov]

Committed r108077: <http://trac.webkit.org/changeset/108077>

[Simon Fraser (smfr)]

This broke the build, and no-one is around on #webkit to deal with it.

[Ilya Tikhonovsky]

Reverted r108077 for reason:

it broke compilation.

Committed r108083: <http://trac.webkit.org/changeset/108083>

[Peter Rybin]

Created attachment 127630 [details]
Patch

[Vsevolod Vlasov]

Committed r108337: <http://trac.webkit.org/changeset/108337>