SkPDF: ASAN fix: don't double ref new SkPDFBitmap objects

SkPDF: ASAN fix: don't double ref new SkPDFBitmap objects

TBR=robertphillips@google.com,tomhudson@google.com,

Committed: https://skia.googlesource.com/skia/+/3d8c33cd835054affab43ac8c96f349147d2e776

[hal.canary, 2015-10-01 17:41:01]

The CQ bit was checked by halcanary@google.com to run a CQ dry run

[hal.canary, 2015-10-01 17:41:09]

The CQ bit was unchecked by halcanary@google.com

[hal.canary, 2015-10-01 17:57:24]

halcanary@google.com changed reviewers:
+ robertphillips@google.com, tomhudson@google.com

[hal.canary, 2015-10-01 17:57:28]

The CQ bit was checked by halcanary@google.com

[commit-bot: I haz the power, 2015-10-01 17:57:39]

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

[commit-bot: I haz the power, 2015-10-01 18:06:29]

Committed patchset #1 (id:1) as
https://skia.googlesource.com/skia/+/3d8c33cd835054affab43ac8c96f349147d2e776

[tomhudson, 2015-10-01 18:24:46]

LGTM, but it isn't clear how this is avoiding a double-ref; looks like more of a
extra-unref fix?

[hal.canary, 2015-10-01 18:33:45]

On 2015/10/01 18:24:46, tomhudson wrote:
> LGTM, but it isn't clear how this is avoiding a double-ref; looks like more of
a
> extra-unref fix?

before I was acting as if `new SkPDFObject` created an object with refcnt 0, not
1.

[hal.canary, 2015-10-01 18:34:09]

TODO:  make SkPDFObject not refcounted.

[tomhudson, 2015-10-01 18:43:13]

Wow, I'd lost track of the length of this function - 140 lines. I wanted to say
"there's no new here", but had to read back a long way to be sure.

On 2015/10/01 18:33:45, Hal Canary wrote:
> On 2015/10/01 18:24:46, tomhudson wrote:
> > LGTM, but it isn't clear how this is avoiding a double-ref; looks like more
of
> a
> > extra-unref fix?
> 
> before I was acting as if `new SkPDFObject` created an object with refcnt 0,
not
> 1.

There's no new here, so I'm still confused.

SkCanon::find*() is presumably always going to return something ref'd (if it
returns nonnull).
If the bitmap isn't found, you're creating it (refcnt 1).
You then pass it to addXObjectResource (which explicitly refs it IFF it hasn't
already been stored as an x-object resource), and autounref it on exit.

It seems safe to assume that it'll only be an XObjectResource if it's already in
the Canon?

Hmm - is this really correct yet?
1. We SkCanon::find*() an object with, say, refcnt 2.
2. addXObjectResource() finds it already in fXObjectResources and so does *NOT*
ref it.
3. autounref on exit decrements the refcnt to 1.