Kill JS_TIMER_MAPARRAY

Kill JS_TIMER_MAPARRAY

R=thestig@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/2a0a26c94ef8131ee29ac4d466c42593cf2ff005

[Tom Sepez, 2015-08-13 20:48:27]

Lei, for review.

[Lei Zhang, 2015-08-13 21:48:24]

https://codereview.chromium.org/1286383004/diff/20001/fpdfsdk/include/javascr...
File fpdfsdk/include/javascript/JS_Object.h (right):

https://codereview.chromium.org/1286383004/diff/20001/fpdfsdk/include/javascr...
fpdfsdk/include/javascript/JS_Object.h:122:
GetGlobalTimerMap()->erase(m_nTimerID);
Aren't you leaking memory here? JS_TIMER_MAPARRAY looks like a
base::ScopedPtrMap to me.

[Tom Sepez, 2015-08-13 21:54:11]

https://codereview.chromium.org/1286383004/diff/20001/fpdfsdk/include/javascr...
File fpdfsdk/include/javascript/JS_Object.h (right):

https://codereview.chromium.org/1286383004/diff/20001/fpdfsdk/include/javascr...
fpdfsdk/include/javascript/JS_Object.h:122:
GetGlobalTimerMap()->erase(m_nTimerID);
On 2015/08/13 21:48:24, Lei Zhang wrote:
> Aren't you leaking memory here? JS_TIMER_MAPARRAY looks like a
> base::ScopedPtrMap to me.

I don't think so.  JS_TIMER_MAPARRAY was an array of pointers to the
intermediate JS_TIMER_MAP, which functions the same as the std::pair in
std::map.  So the delete at old line 134 would be deleting an intermediate
struct (which no longer exists) and not the timer itself.

[Lei Zhang, 2015-08-13 22:10:02]

On 2015/08/13 21:54:11, Tom Sepez (slow until Aug 14) wrote:
>
https://codereview.chromium.org/1286383004/diff/20001/fpdfsdk/include/javascr...
> File fpdfsdk/include/javascript/JS_Object.h (right):
> 
>
https://codereview.chromium.org/1286383004/diff/20001/fpdfsdk/include/javascr...
> fpdfsdk/include/javascript/JS_Object.h:122:
> GetGlobalTimerMap()->erase(m_nTimerID);
> On 2015/08/13 21:48:24, Lei Zhang wrote:
> > Aren't you leaking memory here? JS_TIMER_MAPARRAY looks like a
> > base::ScopedPtrMap to me.
> 
> I don't think so.  JS_TIMER_MAPARRAY was an array of pointers to the
> intermediate JS_TIMER_MAP, which functions the same as the std::pair in
> std::map.  So the delete at old line 134 would be deleting an intermediate
> struct (which no longer exists) and not the timer itself.

*blink* *blink* lgtm

[Tom Sepez, 2015-08-13 22:12:33]

Why don't you land 
https://codereview.chromium.org/1287863002/
first?

[Lei Zhang, 2015-08-13 22:15:52]

On 2015/08/13 22:12:33, Tom Sepez (slow until Aug 14) wrote:
> Why don't you land 
> https://codereview.chromium.org/1287863002/
> first?

Will do shortly.

[Tom Sepez, 2015-08-13 22:55:00]

Committed patchset #4 (id:60001) manually as
2a0a26c94ef8131ee29ac4d466c42593cf2ff005 (presubmit successful).