begin to skiafy PDF headers : removing use of SkRefPtr

begin to skiafy PDF headers : removing use of SkRefPtr
Committed: https://code.google.com/p/skia/source/detail?r=5596

[reed1, 2012-09-19 13:37:33]

sample of the type of change I plan to do.

interestingly, I find zero callers for getAnnotations() in chrome...

[reed1, 2012-09-19 13:50:51]

patch #2 renames getMediaBox() to copyMediaBox() -- since it returns a copy

[reed1, 2012-09-19 16:14:41]

PTAL

This is 1/2 of my goal for this file: it removes SkRefPtr from our public API.
The other half would be to remove references to SkRefPtr completely from the
header, moving me closer to my larger goal of making SkRefPtr private to just
skia (and perhaps gone entirely eventually).

[Steve VanDeBogart, 2012-09-19 16:55:28]

LGTM

On 2012/09/19 13:37:33, reed1 wrote:
> sample of the type of change I plan to do.
> 
> interestingly, I find zero callers for getAnnotations() in chrome...

Yea, the caller is in SkPDFDocument.  Maybe that means it doesn't need SK_API.

On 2012/09/19 16:14:41, reed1 wrote:
> PTAL
> 
> This is 1/2 of my goal for this file: it removes SkRefPtr from our public API.
> The other half would be to remove references to SkRefPtr completely from the
> header, moving me closer to my larger goal of making SkRefPtr private to just
> skia (and perhaps gone entirely eventually).

Last time it came up, the pdf code is the only place where SkRefPtr was used.  I
see no reason why it couldn't be removed entirely.  Unfortunately that kind of
clean up is pretty low on my priority list these day.

https://codereview.appspot.com/6526050/diff/4001/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

https://codereview.appspot.com/6526050/diff/4001/src/pdf/SkPDFDevice.cpp#newc...
src/pdf/SkPDFDevice.cpp:546: SkSafeUnref(fAnnotations);
Move to cleanUp.  It's a bug that it wasn't there before.

https://codereview.appspot.com/6526050/diff/4001/src/pdf/SkPDFDevice.cpp#newc...
src/pdf/SkPDFDevice.cpp:1109: // should this be a singleton?
SkPDFInt(0)?  It wouldn't hurt, but I don't think it'd save much memory.

[reed1, 2012-09-19 17:05:08]

On 2012/09/19 16:55:28, Steve VanDeBogart wrote:
> <span id="abbrex-spnAbbr_365" class="abbrex-Abbr abbrex-Abbr_LGTM
abbrex-AbbrUnknown abbrex-AbbrUnknownVisible">LGTM</span>
> 
> On 2012/09/19 13:37:33, reed1 wrote:
> > sample of the type of change I plan to do.
> > 
> > interestingly, I find zero callers for getAnnotations() in chrome...
> 
> Yea, the caller is in SkPDFDocument.  Maybe that means it doesn't need SK_API.
> 
> On 2012/09/19 16:14:41, reed1 wrote:
> > <span id="abbrex-spnAbbr_366" class="abbrex-Abbr abbrex-Abbr_PTAL
abbrex-AbbrUnknown abbrex-AbbrUnknownVisible">PTAL</span>
> > 
> > This is 1/2 of my goal for this file: it removes SkRefPtr from our public
<span id="abbrex-spnAbbr_367" class="abbrex-Abbr abbrex-Abbr_API
abbrex-AbbrUnknown abbrex-AbbrUnknownVisible">API</span>.
> > The other half would be to remove references to SkRefPtr completely from the
> > header, moving me closer to my larger goal of making SkRefPtr private to
just
> > skia (and perhaps gone entirely eventually).
> 
> Last time it came up, the pdf code is the only place where SkRefPtr was used. 
I
> see no reason why it couldn't be removed entirely.  Unfortunately that kind of
> clean up is pretty low on my priority list these day.
> 
> https://codereview.appspot.com/6526050/diff/4001/src/pdf/SkPDFDevice.cpp
> File src/pdf/SkPDFDevice.cpp (right):
> 
>
https://codereview.appspot.com/6526050/diff/4001/src/pdf/SkPDFDevice.cpp#newc...
> src/pdf/SkPDFDevice.cpp:546: SkSafeUnref(fAnnotations);
> Move to cleanUp.  It's a bug that it wasn't there before.

Done.

> 
>
https://codereview.appspot.com/6526050/diff/4001/src/pdf/SkPDFDevice.cpp#newc...
> src/pdf/SkPDFDevice.cpp:1109: // should this be a singleton?
> SkPDFInt(0)?  It wouldn't hurt, but I don't think it'd save much memory.

[edisonn, 2012-09-19 17:11:52]

lgtm