Introduce pdfium::FakeUniquePtr for keys to sets of unique ptrs.

Introduce pdfium::FakeUniquePtr for keys to sets of unique ptrs.

Committed: https://pdfium.googlesource.com/pdfium/+/a752edfd2ddc4913aeffd31b67f6fdb53e4116ae

[Tom Sepez, 2016-08-18 19:18:03]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-18 19:18:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tom Sepez, 2016-08-18 19:18:35]

tsepez@chromium.org changed reviewers:
+ brucedawson@chromium.org, dsinclair@chromium.org, thestig@chromium.org

[Tom Sepez, 2016-08-18 19:18:53]

CL to see if this compiles everywhere.

[commit-bot: I haz the power, 2016-08-18 19:32:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-18 19:32:24]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-08-19 05:53:12]

lgtm

https://codereview.chromium.org/2262473002/diff/1/fpdfsdk/javascript/app.h
File fpdfsdk/javascript/app.h (right):

https://codereview.chromium.org/2262473002/diff/1/fpdfsdk/javascript/app.h#ne...
fpdfsdk/javascript/app.h:11: #include <set>
Can we go with unordered_set?

[Tom Sepez, 2016-08-19 17:17:03]

@brucedawson - can you think of a reason why this might be evil?

[Tom Sepez, 2016-08-19 17:20:52]

https://codereview.chromium.org/2262473002/diff/1/fpdfsdk/javascript/app.h
File fpdfsdk/javascript/app.h (right):

https://codereview.chromium.org/2262473002/diff/1/fpdfsdk/javascript/app.h#ne...
fpdfsdk/javascript/app.h:11: #include <set>
On 2016/08/19 05:53:12, Lei Zhang wrote:
> Can we go with unordered_set?

Done.

https://codereview.chromium.org/2262473002/diff/1/third_party/base/stl_util.h
File third_party/base/stl_util.h (right):

https://codereview.chromium.org/2262473002/diff/1/third_party/base/stl_util.h...
third_party/base/stl_util.h:37: };
Note: Dan, I didn't stub the other methods, reading the spec made my head hurt.

[brucedawson, 2016-08-19 20:59:46]

It's a bit tricky, but the trickiness is isolated, so probably fine.

https://codereview.chromium.org/2262473002/diff/20001/fpdfsdk/javascript/app.cpp
File fpdfsdk/javascript/app.cpp (right):

https://codereview.chromium.org/2262473002/diff/20001/fpdfsdk/javascript/app....
fpdfsdk/javascript/app.cpp:628:
m_Timers.erase(pdfium::LeakPtr<GlobalTimer>(pTimer));
Can we DCHECK or otherwise verify that this actually finds a timer to erase?

https://codereview.chromium.org/2262473002/diff/20001/third_party/base/stl_ut...
File third_party/base/stl_util.h (right):

https://codereview.chromium.org/2262473002/diff/20001/third_party/base/stl_ut...
third_party/base/stl_util.h:30: // Means of generating a key for
std::set<std::unique_ptr> that avoids
std::set is no longer correct for its first use. Perhaps rephrase? Not crucial,
but maybe something like:

Means of generating a key for searching STL containers that avoids the side
effect of deleting the pointer.

https://codereview.chromium.org/2262473002/diff/20001/third_party/base/stl_ut...
third_party/base/stl_util.h:33: class LeakPtr : public std::unique_ptr<T> {
The name is not intuitively meaningful to me. I'm not sure what would be better.
NownOwningUniquePtr?

[Tom Sepez, 2016-08-19 21:16:43]

https://codereview.chromium.org/2262473002/diff/20001/fpdfsdk/javascript/app.cpp
File fpdfsdk/javascript/app.cpp (right):

https://codereview.chromium.org/2262473002/diff/20001/fpdfsdk/javascript/app....
fpdfsdk/javascript/app.cpp:628:
m_Timers.erase(pdfium::LeakPtr<GlobalTimer>(pTimer));
On 2016/08/19 20:59:46, brucedawson wrote:
> Can we DCHECK or otherwise verify that this actually finds a timer to erase?

Done.

https://codereview.chromium.org/2262473002/diff/20001/third_party/base/stl_ut...
File third_party/base/stl_util.h (right):

https://codereview.chromium.org/2262473002/diff/20001/third_party/base/stl_ut...
third_party/base/stl_util.h:30: // Means of generating a key for
std::set<std::unique_ptr> that avoids
On 2016/08/19 20:59:46, brucedawson wrote:
> std::set is no longer correct for its first use. Perhaps rephrase? Not
crucial,
> but maybe something like:
> 
> Means of generating a key for searching STL containers that avoids the side
> effect of deleting the pointer.

Done.

https://codereview.chromium.org/2262473002/diff/20001/third_party/base/stl_ut...
third_party/base/stl_util.h:33: class LeakPtr : public std::unique_ptr<T> {
On 2016/08/19 20:59:46, brucedawson wrote:
> The name is not intuitively meaningful to me. I'm not sure what would be
better.
> NownOwningUniquePtr?

I've thought of (modulo hyphens and camelCase):

duplicate_ptr
duplicated_ptr
duplicate_unique_ptr
leaky_unique_ptr
non_unique_ptr
fake_unique_ptr
non_deleting_unique_ptr

There's a certain humor to NonUniquePtr(), maybe go with that?

[Tom Sepez, 2016-08-19 21:20:24]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-19 21:20:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[brucedawson, 2016-08-19 21:24:15]

> fake_unique_ptr
> non_deleting_unique_ptr
> 
> There's a certain humor to NonUniquePtr(), maybe go with that?

I like fake_unique_ptr. That clearly suggests that reading the comment
description to get more information would be a good idea, and it 'feels' write
to me.

[Tom Sepez, 2016-08-19 21:29:39]

Description was changed from

==========
Introduce pdfium::LeakPtr for keys to sets of unique ptrs.
==========

to

==========
Introduce pdfium::FakeUniquePtr for keys to sets of unique ptrs.
==========

[Tom Sepez, 2016-08-19 21:29:59]

And so it shall be.

[Tom Sepez, 2016-08-19 21:31:40]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-19 21:31:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[brucedawson, 2016-08-19 21:42:02]

lgtm

[Tom Sepez, 2016-08-19 21:43:59]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2016-08-19 21:43:59]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/2262473002/#ps100001
(title: "Poiner is spelt with a "t"")

[commit-bot: I haz the power, 2016-08-19 21:44:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-19 21:57:44]

Description was changed from

==========
Introduce pdfium::FakeUniquePtr for keys to sets of unique ptrs.
==========

to

==========
Introduce pdfium::FakeUniquePtr for keys to sets of unique ptrs.

Committed:
https://pdfium.googlesource.com/pdfium/+/a752edfd2ddc4913aeffd31b67f6fdb53e41...
==========

[commit-bot: I haz the power, 2016-08-19 21:57:45]

Committed patchset #6 (id:100001) as
https://pdfium.googlesource.com/pdfium/+/a752edfd2ddc4913aeffd31b67f6fdb53e41...