[PDF] Remove one copy of each content stream.

[PDF] Remove one copy of each content stream.

Committed: http://code.google.com/p/skia/source/detail?r=856

[Steve VanDeBogart, 2011-02-24 21:45:36]

Thanks for all the thoughtful reviews today.

[reed1, 2011-02-24 22:27:50]

LGTM

http://codereview.appspot.com/4231044/diff/1/include/pdf/SkPDFDevice.h
File include/pdf/SkPDFDevice.h (right):

http://codereview.appspot.com/4231044/diff/1/include/pdf/SkPDFDevice.h#newcod...
include/pdf/SkPDFDevice.h:138: SkStream* content() const;
Does your comment mean that the contents of the stream might change later on? If
so, we should say that (which is fine by me if that is efficient).

http://codereview.appspot.com/4231044/diff/1/src/pdf/SkPDFFormXObject.cpp
File src/pdf/SkPDFFormXObject.cpp (right):

http://codereview.appspot.com/4231044/diff/1/src/pdf/SkPDFFormXObject.cpp#new...
src/pdf/SkPDFFormXObject.cpp:33: fStream = new SkPDFStream(content.get());
I'm OK with this pattern, as you use it a lot elsewhere, but is the following
equivalent?

SkAutoUnref content(device->content());
fStream = new SkPDFStream((SkStream*)content.get());

If it is, perhaps we should create SkAutoTUnref which takes a type, which would
eliminate the need for the cast above. I think I prefer it because it is only 2
lines, and there is no need to have the explicit "foo->unref(); // SkRefPtr
comment..." each time. Just a thought.

[Steve VanDeBogart, 2011-02-24 23:22:51]

http://codereview.appspot.com/4231044/diff/1/include/pdf/SkPDFDevice.h
File include/pdf/SkPDFDevice.h (right):

http://codereview.appspot.com/4231044/diff/1/include/pdf/SkPDFDevice.h#newcod...
include/pdf/SkPDFDevice.h:138: SkStream* content() const;
On 2011/02/24 22:27:52, reed1 wrote:
> Does your comment mean that the contents of the stream might change later on?
If
> so, we should say that (which is fine by me if that is efficient).

No, I just mean that content returns a new SkStream and the caller has to
unref() it when done.  I changed the comment to try to make it clearer.

http://codereview.appspot.com/4231044/diff/1/src/pdf/SkPDFFormXObject.cpp
File src/pdf/SkPDFFormXObject.cpp (right):

http://codereview.appspot.com/4231044/diff/1/src/pdf/SkPDFFormXObject.cpp#new...
src/pdf/SkPDFFormXObject.cpp:33: fStream = new SkPDFStream(content.get());
On 2011/02/24 22:27:52, reed1 wrote:
> I'm OK with this pattern, as you use it a lot elsewhere, but is the following
> equivalent?
> 
> SkAutoUnref content(device->content());
> fStream = new SkPDFStream((SkStream*)content.get());
> 
> If it is, perhaps we should create SkAutoTUnref which takes a type, which
would
> eliminate the need for the cast above. I think I prefer it because it is only
2
> lines, and there is no need to have the explicit "foo->unref(); // SkRefPtr
> comment..." each time. Just a thought.

SGTM