Bug 59394 – XMLDocumentParserLibxml2 should play nice with strict OwnPtrs

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

ValidationMessage and XMLDocumentParserLibxml2 should play nice with strict OwnPtrs

[Adam Barth]

Created attachment 91006 [details]
Patch

[Adam Barth]

Committed r84835: <http://trac.webkit.org/changeset/84835>

[WebKit Review Bot]

http://trac.webkit.org/changeset/84835 might have broken Chromium Linux Release

[James Robinson]

Reverted r84835 for reason:

Breaks compile because Deque<OwnPtr> doesn't work

Committed r84843: <http://trac.webkit.org/changeset/84843>

[Maciej Stachowiak]

Created attachment 91264 [details]
Patch

[WebKit Review Bot]

Attachment 91264 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8509937

[Maciej Stachowiak]

I guess some of the EWS bots don't like it. I can't guess what would cause that linker error.

[WebKit Review Bot]

Attachment 91264 [details] did not build on mac:
Build output: http://queues.webkit.org/results/8509940

[Build Bot]

Attachment 91264 [details] did not build on win:
Build output: http://queues.webkit.org/results/8513291

[Maciej Stachowiak]

OK, now I think I see why it might be a deque problem and why it only shows up in the linker.

[Maciej Stachowiak]

It's Deque::takeFirst that has the problem and it can easily by avoided by just using first() and removeFirst() separately. It would be hard to avoid otherwise short of specializing for OwnPtr.

[WebKit Review Bot]

Attachment 91264 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8514190

[Adam Barth]

(In reply to comment #11)
> It's Deque::takeFirst that has the problem and it can easily by avoided by just using first() and removeFirst() separately. It would be hard to avoid otherwise short of specializing for OwnPtr.

Ah, ok.  I can pick this up again then.  Thanks!

[Maciej Stachowiak]

Created attachment 91707 [details]
Patch

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/wtf/PassTraits.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/wtf/Deque.h:37:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 10 files


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

[Darin Adler]

Comment on attachment 91707 [details]
Patch

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

>> Source/JavaScriptCore/wtf/Deque.h:37
>> +#include "PassTraits.h"
> 
> Alphabetical sorting problem.  [build/include_order] [4]

What the stylebot said ;-)

> Source/JavaScriptCore/wtf/Deque.h:55
> +        typedef PassTraits<T> Pass;

Not 100% sure the name “Pass” is clear enough name for this. If it wasn’t for the name conflict, I’d suggest calling it PassTraits.

>> Source/JavaScriptCore/wtf/PassTraits.h:41
>> +    template<typename T> struct PassTraits {
> 
> Code inside a namespace should not be indented.  [whitespace/indent] [4]

Would be better to follow the style guide here.

> Source/JavaScriptCore/wtf/PassTraits.h:44
> +        static PassType transfer(Type& t) { return t; }

Is “t” the best name for this argument? Maybe “argument”? Or “object”?

[Patrick R. Gansterer]

Created attachment 93346 [details]
Patch

@mjs: I fixed the style issues of you patch. Can you verify the patch and set cq+?

[Patrick R. Gansterer]

*** Bug 60676 has been marked as a duplicate of this bug. ***

[WebKit Commit Bot]

Comment on attachment 93346 [details]
Patch

Rejecting attachment 93346 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-4', 'apply-..." exit_code: 2

Last 500 characters of output:
----------
|Index: Source/JavaScriptCore/wtf/PassTraits.h
|index 76c4769..3462734 100644
|--- Source/JavaScriptCore/wtf/PassTraits.h
|+++ Source/JavaScriptCore/wtf/PassTraits.h
--------------------------
No file to patch.  Skipping patch.
1 out of 1 hunk ignored
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/dom/XMLDocumentParserLibxml2.cpp

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/8689820

[Patrick R. Gansterer]

Committed r86383: <http://trac.webkit.org/changeset/86383>