Add FX_OVERRIDE and use it for virtual functions of FX_FINAL classes.

Add FX_OVERRIDE and use it for virtual functions of FX_FINAL classes.

Should there be cases where this fails to compile, it indicates a mistake,
either an incorrectly declared overrriden virtual method, or a method that
should be declared non-virtual.

The only issues were with CPDF_CustomAccess::GetBlock(), CPDF_CustomAccess::GetByte(),
and CPDF_CustomAccess::GetFullPath(). These don't appear to be used anywhere,
and are removed.  Two members are removed that are no longer needed once those
methods are removed.

R=jam@chromium.org, jun_fang@foxitsoftware.com

Committed: https://pdfium.googlesource.com/pdfium/+/368ed46

[Tom Sepez, 2014-08-08 20:42:41]

Jun, please take a look.  In trying to trace how the recent fix to GetBlock()
might have affected bug 401580, I noticed that there don't appear to be any
callers of these two methods.  Please confirm.  Thanks.

[Tom Sepez, 2014-08-08 21:18:07]

@jam - is there something special about this interface we have to preserve?
Thanks.

[jun_fang, 2014-08-08 22:16:41]

https://codereview.chromium.org/454983003/diff/20001/fpdfsdk/include/fsdk_def...
File fpdfsdk/include/fsdk_define.h (right):

https://codereview.chromium.org/454983003/diff/20001/fpdfsdk/include/fsdk_def...
fpdfsdk/include/fsdk_define.h:112: virtual void		Release() FX_OVERRIDE { delete
this; }
Shall we consider to remove delete this from pdfium?

https://codereview.chromium.org/454983003/diff/20001/fpdfsdk/src/fpdfview.cpp
File fpdfsdk/src/fpdfview.cpp (right):

https://codereview.chromium.org/454983003/diff/20001/fpdfsdk/src/fpdfview.cpp...
fpdfsdk/src/fpdfview.cpp:14:
CPDF_CustomAccess::CPDF_CustomAccess(FPDF_FILEACCESS* pFileAccess)
Need to check whether pFileAccess is valid(not NULL) before it's accessed?

[Tom Sepez, 2014-08-08 22:21:37]

https://codereview.chromium.org/454983003/diff/20001/fpdfsdk/include/fsdk_def...
File fpdfsdk/include/fsdk_define.h (right):

https://codereview.chromium.org/454983003/diff/20001/fpdfsdk/include/fsdk_def...
fpdfsdk/include/fsdk_define.h:112: virtual void		Release() FX_OVERRIDE { delete
this; }
On 2014/08/08 22:16:40, jun_fang wrote:
> Shall we consider to remove delete this from pdfium?
Maybe later.

https://codereview.chromium.org/454983003/diff/20001/fpdfsdk/src/fpdfview.cpp
File fpdfsdk/src/fpdfview.cpp (right):

https://codereview.chromium.org/454983003/diff/20001/fpdfsdk/src/fpdfview.cpp...
fpdfsdk/src/fpdfview.cpp:14:
CPDF_CustomAccess::CPDF_CustomAccess(FPDF_FILEACCESS* pFileAccess)
On 2014/08/08 22:16:41, jun_fang wrote:
> Need to check whether pFileAccess is valid(not NULL) before it's accessed?
Nah, the existing code seems to work.

[jun_fang, 2014-08-08 22:43:18]

On 2014/08/08 22:21:37, Tom Sepez wrote:
>
https://codereview.chromium.org/454983003/diff/20001/fpdfsdk/include/fsdk_def...
> File fpdfsdk/include/fsdk_define.h (right):
> 
>
https://codereview.chromium.org/454983003/diff/20001/fpdfsdk/include/fsdk_def...
> fpdfsdk/include/fsdk_define.h:112: virtual void		Release() FX_OVERRIDE {
delete
> this; }
> On 2014/08/08 22:16:40, jun_fang wrote:
> > Shall we consider to remove delete this from pdfium?
> Maybe later.
> 
> https://codereview.chromium.org/454983003/diff/20001/fpdfsdk/src/fpdfview.cpp
> File fpdfsdk/src/fpdfview.cpp (right):
> 
>
https://codereview.chromium.org/454983003/diff/20001/fpdfsdk/src/fpdfview.cpp...
> fpdfsdk/src/fpdfview.cpp:14:
> CPDF_CustomAccess::CPDF_CustomAccess(FPDF_FILEACCESS* pFileAccess)
> On 2014/08/08 22:16:41, jun_fang wrote:
> > Need to check whether pFileAccess is valid(not NULL) before it's accessed?
> Nah, the existing code seems to work.

Yes. There are two paths to call CPDF_CustomAccess::CPDF_CustomAccess.
Currently, there are not problems for them. However, this constructor may be
called some whether in future. I suggest to add a check inside
CPDF_CustomAccess. It's a good habit to write defensive code.

[Tom Sepez, 2014-08-08 23:25:10]

Yes. There are two paths to call CPDF_CustomAccess::CPDF_CustomAccess.
> Currently, there are not problems for them. However, this constructor may be
> called some whether in future. I suggest to add a check inside
> CPDF_CustomAccess. It's a good habit to write defensive code.
Done.

[jun_fang, 2014-08-08 23:35:25]

On 2014/08/08 23:25:10, Tom Sepez wrote:
> Yes. There are two paths to call CPDF_CustomAccess::CPDF_CustomAccess.
> > Currently, there are not problems for them. However, this constructor may be
> > called some whether in future. I suggest to add a check inside
> > CPDF_CustomAccess. It's a good habit to write defensive code.
> Done.

LGTM. Thanks!

[jam, 2014-08-12 23:53:31]

On 2014/08/08 21:18:07, Tom Sepez wrote:
> @jam - is there something special about this interface we have to preserve?
> Thanks.

Sorry I'm not sure what you mean?

[jam, 2014-08-12 23:53:32]

On 2014/08/08 21:18:07, Tom Sepez wrote:
> @jam - is there something special about this interface we have to preserve?
> Thanks.

Sorry I'm not sure what you mean?

[Tom Sepez, 2014-08-13 00:08:32]

> Sorry I'm not sure what you mean?
I mean, are there other embedders of pdfium who might be calling these methods
that would break?

[Tom Sepez, 2014-08-13 00:09:55]

> I mean, are there other embedders of pdfium who might be calling these methods
> that would break?
In other words, is fpdfskd/include a "public" public API?

[jam, 2014-08-13 21:38:55]

On 2014/08/13 00:09:55, Tom Sepez wrote:
> > I mean, are there other embedders of pdfium who might be calling these
methods
> > that would break?
> In other words, is fpdfskd/include a "public" public API?

no need to worry about that for now. lgtm

[Tom Sepez, 2014-08-14 00:12:42]

Committed patchset #5 manually as 368ed46 (presubmit successful).