Make more concrete stream classes private to .cpp files

Make more concrete stream classes private to .cpp files

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

[Tom Sepez, 2016-12-01 21:01:31]

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

[commit-bot: I haz the power, 2016-12-01 21:01:42]

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-12-01 21:07:25]

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

[Tom Sepez, 2016-12-01 21:07:25]

npm, ready for review.

[npm, 2016-12-01 21:21:49]

https://codereview.chromium.org/2545653003/diff/1/core/fxcrt/fx_extension.cpp
File core/fxcrt/fx_extension.cpp (right):

https://codereview.chromium.org/2545653003/diff/1/core/fxcrt/fx_extension.cpp...
core/fxcrt/fx_extension.cpp:37: protected:
Can this be private?

https://codereview.chromium.org/2545653003/diff/1/core/fxcrt/fx_extension.cpp...
core/fxcrt/fx_extension.cpp:88: protected:
Ditto

https://codereview.chromium.org/2545653003/diff/1/core/fxcrt/fx_extension.cpp...
core/fxcrt/fx_extension.cpp:96: CFX_CRTFileStream::~CFX_CRTFileStream() {}
Can you place the other methods in CFX_CRTFileStream here?

https://codereview.chromium.org/2545653003/diff/1/core/fxcrt/fx_extension.cpp...
core/fxcrt/fx_extension.cpp:126: protected:
Ditto private?

https://codereview.chromium.org/2545653003/diff/1/core/fxcrt/fx_extension.cpp...
core/fxcrt/fx_extension.cpp:406: }
So now everything above this can go inside namespace?

https://codereview.chromium.org/2545653003/diff/1/fpdfsdk/fpdfview.cpp
File fpdfsdk/fpdfview.cpp (right):

https://codereview.chromium.org/2545653003/diff/1/fpdfsdk/fpdfview.cpp#newcod...
fpdfsdk/fpdfview.cpp:249: class CPDF_CustomAccess final : public
IFX_SeekableReadStream {
Can this go inside namespace?

[commit-bot: I haz the power, 2016-12-01 21:22:13]

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

[commit-bot: I haz the power, 2016-12-01 21:22:14]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-12-01 21:33:45]

https://codereview.chromium.org/2545653003/diff/1/core/fxcrt/fx_extension.cpp
File core/fxcrt/fx_extension.cpp (right):

https://codereview.chromium.org/2545653003/diff/1/core/fxcrt/fx_extension.cpp...
core/fxcrt/fx_extension.cpp:37: protected:
On 2016/12/01 21:21:49, npm wrote:
> Can this be private?

Done.

https://codereview.chromium.org/2545653003/diff/1/core/fxcrt/fx_extension.cpp...
core/fxcrt/fx_extension.cpp:88: protected:
On 2016/12/01 21:21:49, npm wrote:
> Ditto

Done.

https://codereview.chromium.org/2545653003/diff/1/core/fxcrt/fx_extension.cpp...
core/fxcrt/fx_extension.cpp:96: CFX_CRTFileStream::~CFX_CRTFileStream() {}
On 2016/12/01 21:21:49, npm wrote:
> Can you place the other methods in CFX_CRTFileStream here?

Ooops.  Pasted in wrong place.  fixed.

https://codereview.chromium.org/2545653003/diff/1/core/fxcrt/fx_extension.cpp...
core/fxcrt/fx_extension.cpp:126: protected:
On 2016/12/01 21:21:49, npm wrote:
> Ditto private?

Done.  Protected on a final class is always wrong.

https://codereview.chromium.org/2545653003/diff/1/core/fxcrt/fx_extension.cpp...
core/fxcrt/fx_extension.cpp:406: }
On 2016/12/01 21:21:49, npm wrote:
> So now everything above this can go inside namespace?

Probably.  Done.

https://codereview.chromium.org/2545653003/diff/1/fpdfsdk/fpdfview.cpp
File fpdfsdk/fpdfview.cpp (right):

https://codereview.chromium.org/2545653003/diff/1/fpdfsdk/fpdfview.cpp#newcod...
fpdfsdk/fpdfview.cpp:249: class CPDF_CustomAccess final : public
IFX_SeekableReadStream {
On 2016/12/01 21:21:49, npm wrote:
> Can this go inside namespace?

Done.

[npm, 2016-12-01 21:42:21]

lgtm

https://codereview.chromium.org/2545653003/diff/20001/core/fxcrt/fx_extension...
File core/fxcrt/fx_extension.cpp (right):

https://codereview.chromium.org/2545653003/diff/20001/core/fxcrt/fx_extension...
core/fxcrt/fx_extension.cpp:106: if (!nCount) {
Nit: no {}

[Tom Sepez, 2016-12-01 21:43:42]

https://codereview.chromium.org/2545653003/diff/20001/core/fxcrt/fx_extension...
File core/fxcrt/fx_extension.cpp (right):

https://codereview.chromium.org/2545653003/diff/20001/core/fxcrt/fx_extension...
core/fxcrt/fx_extension.cpp:106: if (!nCount) {
On 2016/12/01 21:42:20, npm wrote:
> Nit: no {}

yup.

[Tom Sepez, 2016-12-01 21:43:57]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2016-12-01 21:43:57]

The patchset sent to the CQ was uploaded after l-g-t-m from npm@chromium.org
Link to the patchset: https://codereview.chromium.org/2545653003/#ps40001
(title: "{}")

[commit-bot: I haz the power, 2016-12-01 21:44:06]

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-12-01 21:54:42]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1480628637551420,
"parent_rev": "aeeb6d19d1fc794de482826ecaf8ec2fd9e92d0e", "commit_rev":
"bea049784abf7c32d0b9758fc77d0e601d5c232b"}

[commit-bot: I haz the power, 2016-12-01 21:54:44]

Description was changed from

==========
Make more concrete stream classes private to .cpp files
==========

to

==========
Make more concrete stream classes private to .cpp files

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

[commit-bot: I haz the power, 2016-12-01 21:54:45]

Committed patchset #3 (id:40001) as
https://pdfium.googlesource.com/pdfium/+/bea049784abf7c32d0b9758fc77d0e601d5c...