Bug 121082

Summary:

Use unique_ptr instead of deleteAllValues in XPath

Product:

WebKit

Reporter:

Darin Adler <darin>

Component:

XML

Assignee:

Darin Adler <darin>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andersca, ap, buildbot, cmarcelo, commit-queue, esprehn+autocc, kangil.han, kling, rniwa

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

274872, 73757

Attachments:

Description	Flags
Early incomplete patch	none
Patch	none
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2	none
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion	none
Patch	none
Patch	none
Patch	none
Patch	andersca: review+

Darin Adler

Reported 2013-09-10 00:11:46 PDT

Use OwnPtr instead of deleteAllValues in some of XPath

Attachments
Early incomplete patch (43.60 KB, patch) 2013-09-19 21:22 PDT, Darin Adler	no flags	Details Formatted Diff Diff
Patch (146.79 KB, patch) 2013-10-07 23:29 PDT, Darin Adler	no flags	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (413.75 KB, application/zip) 2013-10-08 00:12 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion (418.64 KB, application/zip) 2013-10-08 00:32 PDT, Build Bot	no flags	Details
Patch (147.42 KB, patch) 2013-10-08 00:48 PDT, Darin Adler	no flags	Details Formatted Diff Diff
Patch (147.66 KB, patch) 2013-10-08 02:12 PDT, Darin Adler	no flags	Details Formatted Diff Diff
Patch (147.57 KB, patch) 2013-10-08 09:23 PDT, Darin Adler	no flags	Details Formatted Diff Diff
Patch (147.61 KB, patch) 2013-10-08 21:59 PDT, Darin Adler	andersca: review+	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Darin Adler

Comment 1 2013-09-19 21:22:30 PDT

Created attachment 212120 [details] Early incomplete patch

Darin Adler

Comment 2 2013-10-07 23:29:42 PDT

Created attachment 213660 [details] Patch

WebKit Commit Bot

Comment 3 2013-10-07 23:32:35 PDT

Attachment 213660 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/dom/Attr.h', u'Source/WebCore/xml/XPathExpression.cpp', u'Source/WebCore/xml/XPathExpression.h', u'Source/WebCore/xml/XPathExpressionNode.cpp', u'Source/WebCore/xml/XPathExpressionNode.h', u'Source/WebCore/xml/XPathFunctions.cpp', u'Source/WebCore/xml/XPathFunctions.h', u'Source/WebCore/xml/XPathGrammar.y', u'Source/WebCore/xml/XPathNodeSet.cpp', u'Source/WebCore/xml/XPathNodeSet.h', u'Source/WebCore/xml/XPathParser.cpp', u'Source/WebCore/xml/XPathParser.h', u'Source/WebCore/xml/XPathPath.cpp', u'Source/WebCore/xml/XPathPath.h', u'Source/WebCore/xml/XPathPredicate.cpp', u'Source/WebCore/xml/XPathPredicate.h', u'Source/WebCore/xml/XPathStep.cpp', u'Source/WebCore/xml/XPathStep.h', u'Source/WebCore/xml/XPathUtil.cpp', u'Source/WebCore/xml/XPathValue.cpp', u'Source/WebCore/xml/XPathValue.h']" exit_code: 1 Source/WebCore/xml/XPathPredicate.h:50: Missing spaces around && [whitespace/operators] [3] Source/WebCore/xml/XPathParser.cpp:45: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/xml/XPathStep.h:74: Missing spaces around && [whitespace/operators] [3] Source/WebCore/xml/XPathStep.h:75: Missing spaces around && [whitespace/operators] [3] Total errors found: 4 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 4 2013-10-08 00:09:10 PDT

Comment on attachment 213660 [details] Patch Attachment 213660 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/3746044

Build Bot

Comment 5 2013-10-08 00:12:38 PDT

Comment on attachment 213660 [details] Patch Attachment 213660 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/3710122 New failing tests: fast/xpath/4XPath/Core/test_core_functions.html fast/xpath/implicit-node-args.html fast/xpath/4XPath/Core/test_boolean_expr.html fast/xpath/union-context-node.xhtml fast/xpath/node-name-case-sensitivity.html inspector/console/console-xpath.html inspector/network-status-non-http.html fast/xpath/xpath-functional-test.html fast/xpath/py-dom-xpath/predicates.html fast/xpath/ambiguous-operators.html fast/xpath/py-dom-xpath/axes.html fast/xpath/substring-non-positive-postion.html fast/xpath/position.html fast/xpath/4XPath/Core/test_nodeset_expr.html fast/xpath/substring-nan-position.html fast/xpath/document-order.html fast/xpath/py-dom-xpath/functions.html fast/xpath/4XPath/Core/test_literal_expr.html inspector/extensions/extensions-audits.html fast/xpath/nan-to-boolean.html fast/xpath/py-dom-xpath/paths.html fast/xpath/py-dom-xpath/expressions.html fast/xpath/xpath-detached-import-assert.html fast/xpath/py-dom-xpath/abbreviations.html fast/xpath/4XPath/Core/test_parser.html fast/xpath/empty-string-substring.html fast/xpath/id-path.html fast/xpath/xpath-detached-nodes.html fast/xpath/4XPath/Core/test_numeric_expr.html fast/xpath/attribute-node-predicate.html

Build Bot

Comment 6 2013-10-08 00:12:40 PDT

Created attachment 213667 [details] Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5

Build Bot

Comment 7 2013-10-08 00:32:56 PDT

Comment on attachment 213660 [details] Patch Attachment 213660 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/3710125 New failing tests: fast/xpath/4XPath/Core/test_core_functions.html fast/xpath/implicit-node-args.html fast/xpath/4XPath/Core/test_boolean_expr.html fast/xpath/union-context-node.xhtml fast/xpath/node-name-case-sensitivity.html inspector/console/console-xpath.html inspector/network-status-non-http.html fast/xpath/xpath-functional-test.html fast/xpath/py-dom-xpath/predicates.html fast/xpath/ambiguous-operators.html fast/xpath/py-dom-xpath/axes.html fast/xpath/substring-non-positive-postion.html fast/xpath/position.html fast/xpath/4XPath/Core/test_nodeset_expr.html fast/xpath/substring-nan-position.html fast/xpath/document-order.html fast/xpath/py-dom-xpath/functions.html fast/xpath/4XPath/Core/test_literal_expr.html inspector/extensions/extensions-audits.html fast/xpath/nan-to-boolean.html fast/xpath/py-dom-xpath/paths.html fast/xpath/py-dom-xpath/expressions.html fast/xpath/xpath-detached-import-assert.html fast/xpath/py-dom-xpath/abbreviations.html fast/xpath/4XPath/Core/test_parser.html fast/xpath/empty-string-substring.html fast/xpath/id-path.html fast/xpath/xpath-detached-nodes.html fast/xpath/4XPath/Core/test_numeric_expr.html fast/xpath/attribute-node-predicate.html

Build Bot

Comment 8 2013-10-08 00:32:59 PDT

Created attachment 213671 [details] Archive of layout-test-results from webkit-ews-02 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.5

Darin Adler

Comment 9 2013-10-08 00:48:52 PDT

Created attachment 213672 [details] Patch

WebKit Commit Bot

Comment 10 2013-10-08 00:50:34 PDT

Attachment 213672 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/dom/Attr.h', u'Source/WebCore/xml/XPathExpression.cpp', u'Source/WebCore/xml/XPathExpression.h', u'Source/WebCore/xml/XPathExpressionNode.cpp', u'Source/WebCore/xml/XPathExpressionNode.h', u'Source/WebCore/xml/XPathFunctions.cpp', u'Source/WebCore/xml/XPathFunctions.h', u'Source/WebCore/xml/XPathGrammar.y', u'Source/WebCore/xml/XPathNodeSet.cpp', u'Source/WebCore/xml/XPathNodeSet.h', u'Source/WebCore/xml/XPathParser.cpp', u'Source/WebCore/xml/XPathParser.h', u'Source/WebCore/xml/XPathPath.cpp', u'Source/WebCore/xml/XPathPath.h', u'Source/WebCore/xml/XPathPredicate.cpp', u'Source/WebCore/xml/XPathPredicate.h', u'Source/WebCore/xml/XPathStep.cpp', u'Source/WebCore/xml/XPathStep.h', u'Source/WebCore/xml/XPathUtil.cpp', u'Source/WebCore/xml/XPathValue.cpp', u'Source/WebCore/xml/XPathValue.h']" exit_code: 1 Source/WebCore/xml/XPathPredicate.h:50: Missing spaces around && [whitespace/operators] [3] Source/WebCore/xml/XPathParser.cpp:45: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/xml/XPathStep.h:74: Missing spaces around && [whitespace/operators] [3] Source/WebCore/xml/XPathStep.h:75: Missing spaces around && [whitespace/operators] [3] Total errors found: 4 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 11 2013-10-08 01:30:22 PDT

Comment on attachment 213672 [details] Patch Attachment 213672 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/3611040

Darin Adler

Comment 12 2013-10-08 02:12:29 PDT

Created attachment 213679 [details] Patch

WebKit Commit Bot

Comment 13 2013-10-08 02:14:03 PDT

Attachment 213679 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/dom/Attr.h', u'Source/WebCore/xml/XPathExpression.cpp', u'Source/WebCore/xml/XPathExpression.h', u'Source/WebCore/xml/XPathExpressionNode.cpp', u'Source/WebCore/xml/XPathExpressionNode.h', u'Source/WebCore/xml/XPathFunctions.cpp', u'Source/WebCore/xml/XPathFunctions.h', u'Source/WebCore/xml/XPathGrammar.y', u'Source/WebCore/xml/XPathNodeSet.cpp', u'Source/WebCore/xml/XPathNodeSet.h', u'Source/WebCore/xml/XPathParser.cpp', u'Source/WebCore/xml/XPathParser.h', u'Source/WebCore/xml/XPathPath.cpp', u'Source/WebCore/xml/XPathPath.h', u'Source/WebCore/xml/XPathPredicate.cpp', u'Source/WebCore/xml/XPathPredicate.h', u'Source/WebCore/xml/XPathStep.cpp', u'Source/WebCore/xml/XPathStep.h', u'Source/WebCore/xml/XPathUtil.cpp', u'Source/WebCore/xml/XPathValue.cpp', u'Source/WebCore/xml/XPathValue.h']" exit_code: 1 Source/WebCore/xml/XPathPredicate.h:50: Missing spaces around && [whitespace/operators] [3] Source/WebCore/xml/XPathParser.cpp:45: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/xml/XPathStep.h:74: Missing spaces around && [whitespace/operators] [3] Source/WebCore/xml/XPathStep.h:75: Missing spaces around && [whitespace/operators] [3] Total errors found: 4 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 14 2013-10-08 03:58:57 PDT

Comment on attachment 213679 [details] Patch Attachment 213679 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/3645201

Darin Adler

Comment 15 2013-10-08 09:23:15 PDT

Created attachment 213693 [details] Patch

WebKit Commit Bot

Comment 16 2013-10-08 09:24:47 PDT

Attachment 213693 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/dom/Attr.h', u'Source/WebCore/xml/XPathExpression.cpp', u'Source/WebCore/xml/XPathExpression.h', u'Source/WebCore/xml/XPathExpressionNode.cpp', u'Source/WebCore/xml/XPathExpressionNode.h', u'Source/WebCore/xml/XPathFunctions.cpp', u'Source/WebCore/xml/XPathFunctions.h', u'Source/WebCore/xml/XPathGrammar.y', u'Source/WebCore/xml/XPathNodeSet.cpp', u'Source/WebCore/xml/XPathNodeSet.h', u'Source/WebCore/xml/XPathParser.cpp', u'Source/WebCore/xml/XPathParser.h', u'Source/WebCore/xml/XPathPath.cpp', u'Source/WebCore/xml/XPathPath.h', u'Source/WebCore/xml/XPathPredicate.cpp', u'Source/WebCore/xml/XPathPredicate.h', u'Source/WebCore/xml/XPathStep.cpp', u'Source/WebCore/xml/XPathStep.h', u'Source/WebCore/xml/XPathUtil.cpp', u'Source/WebCore/xml/XPathValue.cpp', u'Source/WebCore/xml/XPathValue.h']" exit_code: 1 Source/WebCore/xml/XPathPredicate.h:50: Missing spaces around && [whitespace/operators] [3] Source/WebCore/xml/XPathParser.cpp:45: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/xml/XPathStep.h:74: Missing spaces around && [whitespace/operators] [3] Source/WebCore/xml/XPathStep.h:75: Missing spaces around && [whitespace/operators] [3] Total errors found: 4 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 17 2013-10-08 10:06:25 PDT

Comment on attachment 213693 [details] Patch Attachment 213693 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/3747158

Darin Adler

Comment 18 2013-10-08 11:20:36 PDT

Comment on attachment 213693 [details] Patch Anders pointed out that the various arguments should all be passed by value, not rvalue reference. I’ll fix that. And to fix Windows, need to work around a compile issue in the NodeTest class.

Darin Adler

Comment 19 2013-10-08 21:59:51 PDT

Created attachment 213753 [details] Patch

WebKit Commit Bot

Comment 20 2013-10-08 22:01:46 PDT

Attachment 213753 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/dom/Attr.h', u'Source/WebCore/xml/XPathExpression.cpp', u'Source/WebCore/xml/XPathExpression.h', u'Source/WebCore/xml/XPathExpressionNode.cpp', u'Source/WebCore/xml/XPathExpressionNode.h', u'Source/WebCore/xml/XPathFunctions.cpp', u'Source/WebCore/xml/XPathFunctions.h', u'Source/WebCore/xml/XPathGrammar.y', u'Source/WebCore/xml/XPathNodeSet.cpp', u'Source/WebCore/xml/XPathNodeSet.h', u'Source/WebCore/xml/XPathParser.cpp', u'Source/WebCore/xml/XPathParser.h', u'Source/WebCore/xml/XPathPath.cpp', u'Source/WebCore/xml/XPathPath.h', u'Source/WebCore/xml/XPathPredicate.cpp', u'Source/WebCore/xml/XPathPredicate.h', u'Source/WebCore/xml/XPathStep.cpp', u'Source/WebCore/xml/XPathStep.h', u'Source/WebCore/xml/XPathUtil.cpp', u'Source/WebCore/xml/XPathValue.cpp', u'Source/WebCore/xml/XPathValue.h']" exit_code: 1 Source/WebCore/xml/XPathPredicate.h:50: Missing spaces around && [whitespace/operators] [3] Source/WebCore/xml/XPathParser.cpp:45: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.

Darin Adler

Comment 21 2013-10-09 09:57:19 PDT

Comment on attachment 213753 [details] Patch Hooray, builds on Windows. Time to get this reviewed and landed!

Anders Carlsson

Comment 22 2013-10-09 11:05:40 PDT

Comment on attachment 213753 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213753&action=review > Source/WebCore/xml/XPathExpression.h:53 > + XPathExpression(std::unique_ptr<XPath::Expression>); Could make this explicit for good measure. > Source/WebCore/xml/XPathNodeSet.h:39 > + NodeSet(PassRefPtr<Node> node) : m_isSorted(true), m_subtreesAreDisjoint(false), m_nodes(1, node) { } Explicit (if possible).

Darin Adler

Comment 23 2013-10-09 19:45:00 PDT

Committed r157205: <http://trac.webkit.org/changeset/157205>

Alexey Proskuryakov

Comment 24 2013-10-10 10:35:23 PDT

I'm now seeing leaks on LeaksViewer that I think are new: WTF::Mutex::operator new(unsigned long) WebCore::XPath::Step::NodeTest::operator new(unsigned long) xpathyyparse(WebCore::XPath::Parser&) WebCore::XPath::Parser::parseStatement(WTF::String const&, WebCore::XPathNSResolver*, int&) WebCore::XPathExpression::createExpression(WTF::String const&, WebCore::XPathNSResolver*, int&) WebCore::XPathEvaluator::createExpression(WTF::String const&, WebCore::XPathNSResolver*, int&) WebCore::XPathEvaluator::evaluate(WTF::String const&, WebCore::Node*, WebCore::XPathNSResolver*, unsigned short, WebCore::XPathResult*, int&) Darin, will you have the time to look into this? I don't know which test leaks, but almost all of them should be in fast/xpath.

Darin Adler

Comment 25 2013-10-10 10:46:26 PDT

(In reply to comment #24) > Darin, will you have the time to look into this? I don't know which test leaks, but almost all of them should be in fast/xpath. Not today, but tonight.

Alexey Proskuryakov

Comment 26 2013-10-10 11:41:07 PDT

Filed bug 122609. I confirmed locally that the leaks are new.

Darin Adler

Comment 27 2013-12-09 08:48:13 PST

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

Note You need to log in before you can comment on or make changes to this bug.