Remove Release() from CFX_StreamImp, rename to IFX_StreamImp

Remove Release() from CFX_StreamImp, rename to IFX_StreamImp

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

[Tom Sepez, 2016-05-18 22:53:29]

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

[Tom Sepez, 2016-05-18 22:53:30]

Lei, for review.

[Lei Zhang, 2016-05-18 23:02:30]

lgtm

pImps, pImps everwhere.

https://codereview.chromium.org/1998453002/diff/1/xfa/fgas/crt/fgas_stream.cpp
File xfa/fgas/crt/fgas_stream.cpp (right):

https://codereview.chromium.org/1998453002/diff/1/xfa/fgas/crt/fgas_stream.cp...
xfa/fgas/crt/fgas_stream.cpp:21: virtual uint32_t GetAccessModes() const {
return m_dwAccess; }
This doesn't need to be virtual?

https://codereview.chromium.org/1998453002/diff/1/xfa/fgas/crt/fgas_stream.cp...
xfa/fgas/crt/fgas_stream.cpp:38: uint32_t m_dwAccess;
Should be private with GetAccessModes() and a setter method. Getter/setter
probably should be protected.

https://codereview.chromium.org/1998453002/diff/20001/xfa/fgas/crt/fgas_strea...
File xfa/fgas/crt/fgas_stream.cpp (right):

https://codereview.chromium.org/1998453002/diff/20001/xfa/fgas/crt/fgas_strea...
xfa/fgas/crt/fgas_stream.cpp:399: IFX_StreamImp::IFX_StreamImp() : m_dwAccess(0)
{}
Add some blank lines?

https://codereview.chromium.org/1998453002/diff/20001/xfa/fgas/crt/fgas_strea...
xfa/fgas/crt/fgas_stream.cpp:402: if (m_hFile != NULL) {
Omit != NULL

[Tom Sepez, 2016-05-18 23:29:56]

https://codereview.chromium.org/1998453002/diff/1/xfa/fgas/crt/fgas_stream.cpp
File xfa/fgas/crt/fgas_stream.cpp (right):

https://codereview.chromium.org/1998453002/diff/1/xfa/fgas/crt/fgas_stream.cp...
xfa/fgas/crt/fgas_stream.cpp:21: virtual uint32_t GetAccessModes() const {
return m_dwAccess; }
On 2016/05/18 23:02:30, Lei Zhang wrote:
> This doesn't need to be virtual?

Done.

https://codereview.chromium.org/1998453002/diff/1/xfa/fgas/crt/fgas_stream.cp...
xfa/fgas/crt/fgas_stream.cpp:38: uint32_t m_dwAccess;
On 2016/05/18 23:02:30, Lei Zhang wrote:
> Should be private with GetAccessModes() and a setter method. Getter/setter
> probably should be protected.

Done.  PTAL.

https://codereview.chromium.org/1998453002/diff/20001/xfa/fgas/crt/fgas_strea...
File xfa/fgas/crt/fgas_stream.cpp (right):

https://codereview.chromium.org/1998453002/diff/20001/xfa/fgas/crt/fgas_strea...
xfa/fgas/crt/fgas_stream.cpp:399: IFX_StreamImp::IFX_StreamImp() : m_dwAccess(0)
{}
On 2016/05/18 23:02:30, Lei Zhang wrote:
> Add some blank lines?

Done.

https://codereview.chromium.org/1998453002/diff/20001/xfa/fgas/crt/fgas_strea...
xfa/fgas/crt/fgas_stream.cpp:402: if (m_hFile != NULL) {
On 2016/05/18 23:02:30, Lei Zhang wrote:
> Omit != NULL

Done.

[Lei Zhang, 2016-05-18 23:43:06]

https://codereview.chromium.org/1998453002/diff/40001/xfa/fgas/crt/fgas_strea...
File xfa/fgas/crt/fgas_stream.cpp (right):

https://codereview.chromium.org/1998453002/diff/40001/xfa/fgas/crt/fgas_strea...
xfa/fgas/crt/fgas_stream.cpp:35: IFX_StreamImp();
separator after ctor

https://codereview.chromium.org/1998453002/diff/40001/xfa/fgas/crt/fgas_strea...
xfa/fgas/crt/fgas_stream.cpp:833: ASSERT(m_pData && pData && iTotalSize > 0);
!m_pData

[Tom Sepez, 2016-05-18 23:47:04]

https://codereview.chromium.org/1998453002/diff/40001/xfa/fgas/crt/fgas_strea...
File xfa/fgas/crt/fgas_stream.cpp (right):

https://codereview.chromium.org/1998453002/diff/40001/xfa/fgas/crt/fgas_strea...
xfa/fgas/crt/fgas_stream.cpp:35: IFX_StreamImp();
On 2016/05/18 23:43:06, Lei Zhang wrote:
> separator after ctor

Done.

https://codereview.chromium.org/1998453002/diff/40001/xfa/fgas/crt/fgas_strea...
xfa/fgas/crt/fgas_stream.cpp:833: ASSERT(m_pData && pData && iTotalSize > 0);
On 2016/05/18 23:43:06, Lei Zhang wrote:
> !m_pData

good catch. Done.

[Tom Sepez, 2016-05-19 17:04:12]

PTAAL.

[Lei Zhang, 2016-05-19 17:19:38]

lgtm

[Tom Sepez, 2016-05-19 17:28:05]

The CQ bit was checked by tsepez@chromium.org

[commit-bot: I haz the power, 2016-05-19 17:28:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1998453002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1998453002/60001

[commit-bot: I haz the power, 2016-05-19 17:43:40]

Description was changed from

==========
Remove Release() from CFX_StreamImp, rename to IFX_StreamImp
==========

to

==========
Remove Release() from CFX_StreamImp, rename to IFX_StreamImp

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

[commit-bot: I haz the power, 2016-05-19 17:43:41]

Committed patchset #4 (id:60001) as
https://pdfium.googlesource.com/pdfium/+/d89fcb8f68162eef6111ce35133c80b2f7db...