Add FilePath::FinalExtension and RemoveFinalExtension

Add FilePath::FinalExtension and RemoveFinalExtension

Needed for some ".exe" manipulation on Windows.

R=mark@chromium.org
BUG=crashpad:1
Committed: https://chromium.googlesource.com/chromium/mini_chromium/+/45cbbb054ccef2e77d5233aa95c5f2742a23dec1

[Mark Mentovai, 2015-01-20 17:11:53]

I know you’re just bringing over the upstream implementation, but it was a
little cheesey, which is why we didn’t bring it over before.

https://codereview.chromium.org/863583002/diff/1/base/files/file_path.cc
File base/files/file_path.cc (right):

https://codereview.chromium.org/863583002/diff/1/base/files/file_path.cc#newc...
base/files/file_path.cc:84: // of the file name. The position is relative to
BaseName(), not value().
The second sentence should be harsher: this function requires that its path
argument be a basename, not a full path.

https://codereview.chromium.org/863583002/diff/1/base/files/file_path.cc#newc...
base/files/file_path.cc:219: const StringType::size_type dot =
FinalExtensionSeparatorPosition(path_);
This logic winds up calling FinalExtension() twice, which doesn’t seem
necessary. Why not call it once and then lop off the extension based on its
length?

https://codereview.chromium.org/863583002/diff/1/base/files/file_path.h
File base/files/file_path.h (right):

https://codereview.chromium.org/863583002/diff/1/base/files/file_path.h#newco...
base/files/file_path.h:195: // "foo.tar".
I think this comment means “.gz”, not “foo.tar”. RemoveFinalExtension() would be
“foo.tar”.

The comment should also say that extensionless paths will return "". The
upstream copy says so on Extension(), but since we’re not bringing Extension()
over, we should say so here.

https://codereview.chromium.org/863583002/diff/1/base/files/file_path.h#newco...
base/files/file_path.h:196: StringType FinalExtension() const;
Even though upstream’s not WARN_UNUSED_RESULT, it should be, and this could be
here.

[scottmg, 2015-01-20 18:01:28]

https://codereview.chromium.org/863583002/diff/1/base/files/file_path.cc
File base/files/file_path.cc (right):

https://codereview.chromium.org/863583002/diff/1/base/files/file_path.cc#newc...
base/files/file_path.cc:84: // of the file name. The position is relative to
BaseName(), not value().
On 2015/01/20 17:11:53, Mark Mentovai wrote:
> The second sentence should be harsher: this function requires that its path
> argument be a basename, not a full path.

After simplifying below, just deleted this function.

https://codereview.chromium.org/863583002/diff/1/base/files/file_path.cc#newc...
base/files/file_path.cc:219: const StringType::size_type dot =
FinalExtensionSeparatorPosition(path_);
On 2015/01/20 17:11:53, Mark Mentovai wrote:
> This logic winds up calling FinalExtension() twice, which doesn’t seem
> necessary. Why not call it once and then lop off the extension based on its
> length?

Done.

https://codereview.chromium.org/863583002/diff/1/base/files/file_path.h
File base/files/file_path.h (right):

https://codereview.chromium.org/863583002/diff/1/base/files/file_path.h#newco...
base/files/file_path.h:195: // "foo.tar".
On 2015/01/20 17:11:53, Mark Mentovai wrote:
> I think this comment means “.gz”, not “foo.tar”. RemoveFinalExtension() would
be
> “foo.tar”.
> 
> The comment should also say that extensionless paths will return "". The
> upstream copy says so on Extension(), but since we’re not bringing Extension()
> over, we should say so here.

Done.

https://codereview.chromium.org/863583002/diff/1/base/files/file_path.h#newco...
base/files/file_path.h:196: StringType FinalExtension() const;
On 2015/01/20 17:11:53, Mark Mentovai wrote:
> Even though upstream’s not WARN_UNUSED_RESULT, it should be, and this could be
> here.

I think generally only the FilePath returning ones are marked that way as in
that case it can be unclear if it's mutating or copying. But I guess there's no
useful reason to call FinalExtension() and ignore the result either.

[Mark Mentovai, 2015-01-20 18:07:07]

LGTM

[scottmg, 2015-01-20 18:10:55]

Committed patchset #2 (id:20001) manually as
45cbbb054ccef2e77d5233aa95c5f2742a23dec1 (presubmit successful).