PDF: Fix uninit memory access in PDFiumEngine.

PDF: Fix uninit memory access in PDFiumEngine.

This regressed in r294564.

BUG=413850
Committed: https://crrev.com/d2439d91ce1494ab999c52e23b8284f26c40f954
Cr-Commit-Position: refs/heads/master@{#294854}

[Lei Zhang, 2014-09-12 22:49:03]

thestig@chromium.org changed reviewers:
+ n.bansal@samsung.org, raymes@chromium.org

[Lei Zhang, 2014-09-12 22:49:04]

Since I didn't review the previous CL, I get to fix it instead.

https://codereview.chromium.org/568803004/diff/1/pdf/pdfium/pdfium_engine.h
File pdf/pdfium/pdfium_engine.h (left):

https://codereview.chromium.org/568803004/diff/1/pdf/pdfium/pdfium_engine.h#o...
pdf/pdfium/pdfium_engine.h:513: struct MouseDownState {
non-trivial struct/class -> class

https://codereview.chromium.org/568803004/diff/1/pdf/pdfium/pdfium_engine.h#o...
pdf/pdfium/pdfium_engine.h:514: MouseDownState() {};
didn't initialize |area_| or |target_| -> :(

https://codereview.chromium.org/568803004/diff/1/pdf/pdfium/pdfium_engine.h#o...
pdf/pdfium/pdfium_engine.h:521: return (area_ == rhs.area_) && (target_.url ==
rhs.target_.url);
what about target_.page ??

https://codereview.chromium.org/568803004/diff/1/pdf/pdfium/pdfium_engine.h#o...
pdf/pdfium/pdfium_engine.h:524: bool operator!=(const MouseDownState rhs) const
{
const ref

https://codereview.chromium.org/568803004/diff/1/pdf/pdfium/pdfium_engine.h#o...
pdf/pdfium/pdfium_engine.h:525: return (area_ != rhs.area_) || (target_.url !=
rhs.target_.url);
As written, this could have just been: return !(*this == rhs)

[Nikhil, 2014-09-13 08:22:18]

On 2014/09/12 22:49:04, Lei Zhang wrote:
> Since I didn't review the previous CL, I get to fix it instead.
> 

Sorry about that, my CL is the culprit :(

Also, I'm not sure whose id is n.bansal@samsung.org (added in reviewers). Mine
is n.bansal@samsung.com (never knew there was an .org as well).

[Lei Zhang, 2014-09-13 17:32:40]

On 2014/09/13 08:22:18, Nikhil wrote:
> On 2014/09/12 22:49:04, Lei Zhang wrote:
> > Since I didn't review the previous CL, I get to fix it instead.
> > 
> 
> Sorry about that, my CL is the culprit :(
> 
> Also, I'm not sure whose id is mailto:n.bansal@samsung.org (added in
reviewers). Mine
> is mailto:n.bansal@samsung.com (never knew there was an .org as well).

Yikes, sorry about that. I thought your email should have been unique enough to
trust the autocomplete.

[Lei Zhang, 2014-09-13 17:32:50]

thestig@chromium.org changed reviewers:
- n.bansal@samsung.org

[raymes, 2014-09-15 00:02:59]

lgtm 

Thanks for the fix - sorry for not reviewing the class carefully.

https://codereview.chromium.org/568803004/diff/1/pdf/pdfium/pdfium_engine.cc
File pdf/pdfium/pdfium_engine.cc (right):

https://codereview.chromium.org/568803004/diff/1/pdf/pdfium/pdfium_engine.cc#...
pdf/pdfium/pdfium_engine.cc:1337: return true;
optional: I guess you might as well move this down into OnMouseUp as well?

[Lei Zhang, 2014-09-15 15:26:10]

https://codereview.chromium.org/568803004/diff/1/pdf/pdfium/pdfium_engine.cc
File pdf/pdfium/pdfium_engine.cc (right):

https://codereview.chromium.org/568803004/diff/1/pdf/pdfium/pdfium_engine.cc#...
pdf/pdfium/pdfium_engine.cc:1337: return true;
On 2014/09/15 00:02:59, raymes wrote:
> optional: I guess you might as well move this down into OnMouseUp as well?

Maybe in a separate CL.

[Lei Zhang, 2014-09-15 15:26:14]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2014-09-15 15:26:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/568803004/1

[commit-bot: I haz the power, 2014-09-15 18:42:00]

Committed patchset #1 (id:1) as 58218a2f0f2dfe9600e100fec750ba69c7d0aafd

[commit-bot: I haz the power, 2014-09-15 18:58:21]

Patchset 1 (id:??) landed as
https://crrev.com/d2439d91ce1494ab999c52e23b8284f26c40f954
Cr-Commit-Position: refs/heads/master@{#294854}

[Lei Zhang, 2017-05-10 01:02:42]

https://codereview.chromium.org/568803004/diff/1/pdf/pdfium/pdfium_engine.cc
File pdf/pdfium/pdfium_engine.cc (right):

https://codereview.chromium.org/568803004/diff/1/pdf/pdfium/pdfium_engine.cc#...
pdf/pdfium/pdfium_engine.cc:1337: return true;
On 2014/09/15 15:26:09, Lei Zhang wrote:
> On 2014/09/15 00:02:59, raymes wrote:
> > optional: I guess you might as well move this down into OnMouseUp as well?
> 
> Maybe in a separate CL.

I forgot about this, but I was revisiting this CL, noticed this and filed
https://bugs.chromium.org/p/chromium/issues/detail?id=720179