121082 – Use unique_ptr instead of deleteAllValues in XPath

Bug 121082 - Use unique_ptr instead of deleteAllValues in XPath

Summary: Use unique_ptr instead of deleteAllValues in XPath

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	XML (show other bugs)
Version:	528+ (Nightly build)
Hardware:	All All

Importance:	P2 Normal
Assignee:	Darin Adler

URL:
Keywords:

Duplicates (1):	125389 (view as bug list)
Depends on:
Blocks:	73757
	Show dependency tree / graph

Reported:	2013-09-10 00:11 PDT by Darin Adler
Modified:	2013-12-09 08:48 PST (History)
CC List:	9 users (show)

See Also:

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 an attachment (proposed patch, testcase, etc.)

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

Description Darin Adler 2013-09-10 00:11:46 PDT

Use OwnPtr instead of deleteAllValues in some of XPath

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

Created attachment 212120 [details]
Early incomplete patch

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

Created attachment 213660 [details]
Patch

Comment 3 WebKit Commit Bot 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.

Comment 4 Build Bot 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

Comment 5 Build Bot 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

Comment 6 Build Bot 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

Comment 7 Build Bot 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

Comment 8 Build Bot 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

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

Created attachment 213672 [details]
Patch

Comment 10 WebKit Commit Bot 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.

Comment 11 Build Bot 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

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

Created attachment 213679 [details]
Patch

Comment 13 WebKit Commit Bot 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.

Comment 14 Build Bot 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

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

Created attachment 213693 [details]
Patch

Comment 16 WebKit Commit Bot 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.

Comment 17 Build Bot 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

Comment 18 Darin Adler 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.

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

Created attachment 213753 [details]
Patch

Comment 20 WebKit Commit Bot 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.

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

Comment on attachment 213753 [details]
Patch

Hooray, builds on Windows. Time to get this reviewed and landed!

Comment 22 Anders Carlsson 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).

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

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

Comment 24 Alexey Proskuryakov 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.

Comment 25 Darin Adler 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.

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

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

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

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