Tidy fpdfsave.cpp

Tidy fpdfsave.cpp

Remove CFX_PtrArray filelist.
Promote ScopedFileStream to .h file and use it.
Fix _CAPS names, bool returns, and put in anonymous namespace.
FX_CreateMemoryStream() can't return null, so remove checks.

R=thestig@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/7a73effb9fe071fbc852b30865d7a332e96fbd56

[Tom Sepez, 2016-02-05 22:43:49]

Description was changed from

==========
Tidy fpdfsave.cpp

Promote ScopedFileStream to .h file.
==========

to

==========
Tidy fpdfsave.cpp

Promote ScopedFileStream to .h file.
Fix _CAPS function names and put in anonymous namespace.
==========

[Tom Sepez, 2016-02-05 22:45:16]

Description was changed from

==========
Tidy fpdfsave.cpp

Promote ScopedFileStream to .h file.
Fix _CAPS function names and put in anonymous namespace.
==========

to

==========
Tidy fpdfsave.cpp

Remove CFX_PtrArray filelist.
Promote ScopedFileStream to .h file.
Fix _CAPS names, bool returns, and put in anonymous namespace.
==========

[Tom Sepez, 2016-02-05 22:45:28]

Description was changed from

==========
Tidy fpdfsave.cpp

Remove CFX_PtrArray filelist.
Promote ScopedFileStream to .h file.
Fix _CAPS names, bool returns, and put in anonymous namespace.
==========

to

==========
Tidy fpdfsave.cpp

Remove CFX_PtrArray filelist.
Promote ScopedFileStream to .h file and use it.
Fix _CAPS names, bool returns, and put in anonymous namespace.
==========

[Tom Sepez, 2016-02-05 22:47:00]

Description was changed from

==========
Tidy fpdfsave.cpp

Remove CFX_PtrArray filelist.
Promote ScopedFileStream to .h file and use it.
Fix _CAPS names, bool returns, and put in anonymous namespace.
==========

to

==========
Tidy fpdfsave.cpp

Remove CFX_PtrArray filelist.
Promote ScopedFileStream to .h file and use it.
Fix _CAPS names, bool returns, and put in anonymous namespace.
FX_CreateMemoryStream() can't return null, so remove checks.
==========

[Tom Sepez, 2016-02-05 22:49:44]

tsepez@chromium.org changed reviewers:
+ thestig@chromium.org

[Tom Sepez, 2016-02-05 22:49:45]

Lei, pls review.  One less PtrArray picked up a bunch of other irresistible
changes.

https://codereview.chromium.org/1672153002/diff/1/fpdfsdk/src/fpdfsave.cpp
File fpdfsdk/src/fpdfsave.cpp (left):

https://codereview.chromium.org/1672153002/diff/1/fpdfsdk/src/fpdfsave.cpp#ol...
fpdfsdk/src/fpdfsave.cpp:201: pfileWrite > 0) {
This had to be wrong. Comparing a pointer greater than 0 ?  I've duplicated the
logic from above.  Yay cut n paste.

https://codereview.chromium.org/1672153002/diff/1/xfa/src/fxfa/src/parser/xfa...
File xfa/src/fxfa/src/parser/xfa_object_imp.cpp (left):

https://codereview.chromium.org/1672153002/diff/1/xfa/src/fxfa/src/parser/xfa...
xfa/src/fxfa/src/parser/xfa_object_imp.cpp:1116: if (pMemoryStream) {
Good thing we test the complement of line 1112 ...

[Lei Zhang, 2016-02-05 23:22:43]

https://codereview.chromium.org/1672153002/diff/1/core/include/fpdfapi/fpdf_p...
File core/include/fpdfapi/fpdf_parser.h (right):

https://codereview.chromium.org/1672153002/diff/1/core/include/fpdfapi/fpdf_p...
core/include/fpdfapi/fpdf_parser.h:43: // TODO(thestig) Using unique_ptr with
ReleaseDeleter is still not ideal.
Shall we move this to fx_stream.h where IFX_FileStream lives?

[Lei Zhang, 2016-02-05 23:32:50]

https://codereview.chromium.org/1672153002/diff/1/fpdfsdk/src/fpdfsave.cpp
File fpdfsdk/src/fpdfsave.cpp (right):

https://codereview.chromium.org/1672153002/diff/1/fpdfsdk/src/fpdfsave.cpp#ne...
fpdfsdk/src/fpdfsave.cpp:283: std::vector<ScopedFileStream> fileList;
Do we actually need to hold on to the file streams?

[Tom Sepez, 2016-02-08 17:10:20]

https://codereview.chromium.org/1672153002/diff/1/fpdfsdk/src/fpdfsave.cpp
File fpdfsdk/src/fpdfsave.cpp (right):

https://codereview.chromium.org/1672153002/diff/1/fpdfsdk/src/fpdfsave.cpp#ne...
fpdfsdk/src/fpdfsave.cpp:283: std::vector<ScopedFileStream> fileList;
On 2016/02/05 23:32:49, Lei Zhang wrote:
> Do we actually need to hold on to the file streams?

I didn't think so ... given the loop at old line 309 that released them all ...

[Lei Zhang, 2016-02-08 20:10:29]

So do you want to keep the ScopedFileStreamdef  where it is?

https://codereview.chromium.org/1672153002/diff/1/fpdfsdk/src/fpdfsave.cpp
File fpdfsdk/src/fpdfsave.cpp (right):

https://codereview.chromium.org/1672153002/diff/1/fpdfsdk/src/fpdfsave.cpp#ne...
fpdfsdk/src/fpdfsave.cpp:283: std::vector<ScopedFileStream> fileList;
On 2016/02/08 17:10:20, Tom Sepez wrote:
> On 2016/02/05 23:32:49, Lei Zhang wrote:
> > Do we actually need to hold on to the file streams?
> 
> I didn't think so ... given the loop at old line 309 that released them all
...

What I was wonder is, do we need to hold on to the file streams for the entire
duration. I'm guessing the answer is probably yes, so the code is fine as is.

[Lei Zhang, 2016-02-08 20:10:50]

On 2016/02/08 20:10:29, Lei Zhang wrote:
> So do you want to keep the ScopedFileStreamdef  where it is?

*ScopedFileStream def*

[Tom Sepez, 2016-02-08 20:45:35]

> Shall we move this to fx_stream.h where IFX_FileStream lives?

Currentyly, there isn't any unique_ptr usage in fxcrt, which is why put it here.

[Lei Zhang, 2016-02-08 21:30:00]

ok, lgtm

[Tom Sepez, 2016-02-08 21:40:00]

Description was changed from

==========
Tidy fpdfsave.cpp

Remove CFX_PtrArray filelist.
Promote ScopedFileStream to .h file and use it.
Fix _CAPS names, bool returns, and put in anonymous namespace.
FX_CreateMemoryStream() can't return null, so remove checks.
==========

to

==========
Tidy fpdfsave.cpp

Remove CFX_PtrArray filelist.
Promote ScopedFileStream to .h file and use it.
Fix _CAPS names, bool returns, and put in anonymous namespace.
FX_CreateMemoryStream() can't return null, so remove checks.

R=thestig@chromium.org

Committed:
https://pdfium.googlesource.com/pdfium/+/7a73effb9fe071fbc852b30865d7a332e96f...
==========

[Tom Sepez, 2016-02-08 21:40:01]

Committed patchset #1 (id:1) manually as
7a73effb9fe071fbc852b30865d7a332e96fbd56 (presubmit successful).