Tidy up JS_Object.h and JS_Object.cpp.

Tidy up JS_Object.h and JS_Object.cpp.

R=thestig@chromium.org

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

[Tom Sepez, 2015-08-13 23:13:38]

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

[Tom Sepez, 2015-08-13 23:13:38]

Misc tidying
- use unique_ptr.
- put code in .cpp file.
- make global function into private static method.

[Lei Zhang, 2015-08-13 23:22:24]

https://codereview.chromium.org/1293673003/diff/1/fpdfsdk/include/javascript/...
File fpdfsdk/include/javascript/JS_Object.h (right):

https://codereview.chromium.org/1293673003/diff/1/fpdfsdk/include/javascript/...
fpdfsdk/include/javascript/JS_Object.h:126: static TimerMap*
GetGlobalTimerMap();
Just make this a function in an anonymous namespace within JS_Object.cpp? Move
the "using" statement to the .cpp file as well?

https://codereview.chromium.org/1293673003/diff/1/fpdfsdk/src/javascript/JS_O...
File fpdfsdk/src/javascript/JS_Object.cpp (right):

https://codereview.chromium.org/1293673003/diff/1/fpdfsdk/src/javascript/JS_O...
fpdfsdk/src/javascript/JS_Object.cpp:152: pTimer->m_bProcessing = TRUE;
Not sure if you care to use CFX_AutoRestorer here.

[Tom Sepez, 2015-08-13 23:28:31]

https://codereview.chromium.org/1293673003/diff/1/fpdfsdk/include/javascript/...
File fpdfsdk/include/javascript/JS_Object.h (right):

https://codereview.chromium.org/1293673003/diff/1/fpdfsdk/include/javascript/...
fpdfsdk/include/javascript/JS_Object.h:126: static TimerMap*
GetGlobalTimerMap();
On 2015/08/13 23:22:24, Lei Zhang wrote:
> Just make this a function in an anonymous namespace within JS_Object.cpp? Move
> the "using" statement to the .cpp file as well?
Nah, I like restricting callers to this class, and using a shorter name scoped
by the class.  Let me know if you want to insist.

https://codereview.chromium.org/1293673003/diff/1/fpdfsdk/src/javascript/JS_O...
File fpdfsdk/src/javascript/JS_Object.cpp (right):

https://codereview.chromium.org/1293673003/diff/1/fpdfsdk/src/javascript/JS_O...
fpdfsdk/src/javascript/JS_Object.cpp:152: pTimer->m_bProcessing = TRUE;
On 2015/08/13 23:22:24, Lei Zhang wrote:
> Not sure if you care to use CFX_AutoRestorer here.
Doesn't seem to have the return paths complexity to warrant this (yet).

[Lei Zhang, 2015-08-13 23:36:48]

lgtm

https://codereview.chromium.org/1293673003/diff/1/fpdfsdk/include/javascript/...
File fpdfsdk/include/javascript/JS_Object.h (right):

https://codereview.chromium.org/1293673003/diff/1/fpdfsdk/include/javascript/...
fpdfsdk/include/javascript/JS_Object.h:126: static TimerMap*
GetGlobalTimerMap();
On 2015/08/13 23:28:31, Tom Sepez (slow until Aug 14) wrote:
> On 2015/08/13 23:22:24, Lei Zhang wrote:
> > Just make this a function in an anonymous namespace within JS_Object.cpp?
Move
> > the "using" statement to the .cpp file as well?
> Nah, I like restricting callers to this class, and using a shorter name scoped
> by the class.  Let me know if you want to insist.

Oh, I'm too used to having just 1 class per file.

[Tom Sepez, 2015-08-13 23:43:13]

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