Hi pkasting@,
Given that you have already experience with this presubmit, I would value your reviews on the CLs for this issue. So consider this a heads-up. :)

Also, what do you think of downgrading this from a general error to just an upload error? The check can apparently have false-positives, and in its current form causes issues with landing innocent CLs (e.g., https://crrev.com/c/989952).

The string of CLs is: https://crrev.com/c/989972, https://crrev.com/c/989735, https://crrev.com/c/990132, https://crrev.com/c/990133

@pkasting -- I intend to send them all to you for review (+ then also to OWNERS for rubber-stamp), unless you object to that.

I'm not a good reviewer for these since I don't actually speak either regular expressions or Python (it took me a long time and multiple other people's help to write the change I made here).

I suggest having these reviewed by appropriate OWNERS.

SGTM, thanks!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/52e18bf99289bec27038fb68360d60e751a4d83b

commit 52e18bf99289bec27038fb68360d60e751a4d83b
Author: Vaclav Brozek <vabr@chromium.org>
Date: Tue Apr 03 07:05:24 2018

Fix _CheckUniquePtr

The PRESUBMIT test _CheckUniquePtr reported the following
false-positive:
std::unique_ptr<ParseResult> result = std::make_unique<ParseResult>();
suggesting that one uses nullptr instead.

This CL fixes that bug and also adds regression tests for this
presubmit check.

Bug:  827961 
Change-Id: I60a088f6590f01c51be7e3ffc0c6d65ad9e5c329
Reviewed-on: https://chromium-review.googlesource.com/989972
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547635}
[modify] https://crrev.com/52e18bf99289bec27038fb68360d60e751a4d83b/PRESUBMIT.py
[modify] https://crrev.com/52e18bf99289bec27038fb68360d60e751a4d83b/PRESUBMIT_test.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/95face6560db32aed3f3cf5e69579090be8638c9

commit 95face6560db32aed3f3cf5e69579090be8638c9
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Apr 04 14:15:11 2018

Extend presubmit _CheckUniquePtr to multiline

The presubmit check _CheckUniquePtr guards against calling
std::unique_ptr constructor directly, instead directing code authors to
use std::make_unique.

This check currently fails to match multiline expressions. While it
catches
bar = std::unique_ptr<T>(foo);
it does not catch
bar =
    std::unique_ptr<T>(foo);
nor does it catch
bar = std::unique_ptr<T>(
    foo);

This CL fixes it by extending the match pattern to catch all lines with
the substring "std::unique_ptr<T>(". (But not those with
"std::unique_ptr<T>()", which should be handled by the "nullptr-check".)

Bug:  827961 
Change-Id: I376b5e9811418205e294e97de0b6b7bcbf6891d2
Reviewed-on: https://chromium-review.googlesource.com/989735
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548045}
[modify] https://crrev.com/95face6560db32aed3f3cf5e69579090be8638c9/PRESUBMIT.py
[modify] https://crrev.com/95face6560db32aed3f3cf5e69579090be8638c9/PRESUBMIT_test.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/851d9604e6b04e2213fe110a2ad3cd537f2fddc5

commit 851d9604e6b04e2213fe110a2ad3cd537f2fddc5
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Apr 04 16:13:05 2018

Reduce _CheckUniquePtr spam

Currently, _CheckUniquePtr PRESUBMIT check reports each failing line as
a separate error. This repeats the error explanation and thus clutters
the output necessarily.

This CL makes _CheckUniquePtr collect all occurences of the two issues
it checks for (direct use of unique_ptr constructor and replaceability
with nullptr) and group them under a separate single error, one for each
of the both types of check.

It also adds the failing line into the output, to make it easier to
understand the issue already from the presubmit logs.

This follows what is done for other checks, e.g., _CheckNoPragmaOnce).

Bug:  827961 
Change-Id: Ic7d60a05b6f96da741f1401422f4a1690bb6e279
Reviewed-on: https://chromium-review.googlesource.com/990132
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548081}
[modify] https://crrev.com/851d9604e6b04e2213fe110a2ad3cd537f2fddc5/PRESUBMIT.py
[modify] https://crrev.com/851d9604e6b04e2213fe110a2ad3cd537f2fddc5/PRESUBMIT_test.py

I got the following presubmit error which seems to be a false positive:
** Presubmit ERRORS **
The following files use std::unique_ptr<T>(). Use nullptr instead.
  components/offline_pages/core/model/get_pages_task.cc:406
    return std::unique_ptr<GetPagesTask>(

Could you please make it as a warning first? Thanks.

@8: That looks like a true positive to me, not a false positive.

Replace things like this:

  return std::unique_ptr<T>(new T(...));

With this:

  return std::make_unique<T>(...);

The message should not say "Use nullptr instead". It is confusing. 

It's supposed to say "Use std::make_unique<T>() instead." in that case.  Sounds like a bug that it doesn't.

Issue 829430 has been merged into this issue.

Thanks for the feedback, jianli@.

As for making this a warning -- I have https://crrev.com/c/990133 in review since Wednesday, will try to ping jochen@ to approve that today.

I'll look into the confusing message, and also into the false-positive reported in bug 829430.

For more context -- I was bitten by one of this test's false-positive, just like you. That's how I came to fixing it. :)

The confusing message is https://crrev.com/c/999604.
Now I'm looking into the error reported in bug 829430.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea41ab27ab25480ab261b517e23c558a0d91acf3

commit ea41ab27ab25480ab261b517e23c558a0d91acf3
Author: Vaclav Brozek <vabr@chromium.org>
Date: Fri Apr 06 13:21:53 2018

Make _CheckUniquePtr an upload-only check

Currently, _CheckUniquePtr is run both on upload and on commit. This
check emits errors, which means commit is blocked if it fires. At the
same time, it has a history of false positives (one mentioned in
https://crrev.com/c/933547, another in  https://crbug.com/827961 ).

Therefore this CL makes _CheckUniquePtr an upload-only check.

Bug:  827961 
Change-Id: I6d5a7f3dda33432ae1375359c2ae573e5ad3df34
Reviewed-on: https://chromium-review.googlesource.com/990133
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548760}
[modify] https://crrev.com/ea41ab27ab25480ab261b517e23c558a0d91acf3/PRESUBMIT.py

And the error from bug 829430 is getting fixed in https://crrev.com/c/998194.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c2fecf47e6516b1ef160c13d164883b9de742bd7

commit c2fecf47e6516b1ef160c13d164883b9de742bd7
Author: Vaclav Brozek <vabr@chromium.org>
Date: Fri Apr 06 16:40:16 2018

Fix swapped messages in _CheckUniquePtr

_CheckUniquePtr consistes of two checks -- one related to nullptr, one
related to make_unique. The error messages are currently swapped by
accident. This CL fixes that and adds a check for that.

It also removes some confusing "java/src" subpaths from the mock file
paths in the check. Those got in via copy&paste and are confusing,
while not relevant for what the test is testing.

Bug:  827961 
Change-Id: Iaf3272cfc5fe0a641feaee861ad17fe71505800b
Reviewed-on: https://chromium-review.googlesource.com/999604
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548807}
[modify] https://crrev.com/c2fecf47e6516b1ef160c13d164883b9de742bd7/PRESUBMIT.py
[modify] https://crrev.com/c2fecf47e6516b1ef160c13d164883b9de742bd7/PRESUBMIT_test.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a54c528b528a68eb0e8e7aa5383aa3e6f1609dba

commit a54c528b528a68eb0e8e7aa5383aa3e6f1609dba
Author: Vaclav Brozek <vabr@chromium.org>
Date: Fri Apr 06 19:23:55 2018

Fix another false-positive of _CheckUniquePtr

The false-positive was caused by an error in matching <...> after
unique_ptr.  The old expression essentially said:
<.*?>\(
The "?", making "*" non-greedy, was meant to prevent ".*" from
matching more than one <...> block.
But that does not work -- . would still match the '>' which ended the
first block and carry on until an '>(' was found, if necessary.

So in the concrete example:
std::unique_ptr<T> result = std::make_unique<T>(
what the presubmit check "though" was the the type parameter was
"T> result = std::make_unique<T", instead of just "T".

A better way to ensure matching just one <...> block was already used
in the sibling check pattern (null_construct_pattern) -- splitting the
(possibly nested) <...> block into the part containing <s and the part
containing >s (note that a regular expression cannot check that both
parts have a matching number of < and >).

This CL just adopted that better technique for the faulty pattern.

Also remove a temporary hack from webstore_provider.cc added to bypass
the failing check.

Bug: 733662,  827961 
Change-Id: Ia70cc4333f8afc4d45b1f676ea1bc870f6a3a079
Reviewed-on: https://chromium-review.googlesource.com/998194
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548896}
[modify] https://crrev.com/a54c528b528a68eb0e8e7aa5383aa3e6f1609dba/PRESUBMIT.py
[modify] https://crrev.com/a54c528b528a68eb0e8e7aa5383aa3e6f1609dba/PRESUBMIT_test.py
[modify] https://crrev.com/a54c528b528a68eb0e8e7aa5383aa3e6f1609dba/chrome/browser/ui/app_list/search/arc_app_result.cc
[modify] https://crrev.com/a54c528b528a68eb0e8e7aa5383aa3e6f1609dba/chrome/browser/ui/app_list/search/webstore/webstore_provider.cc

The planned fixes and also some unplanned ones are all in, so marking as fixed. Will reopen if there are more breakages or if I receive requests for follow-ups.