[webkit-reviews] review requested: [Bug 3527] Allow Safari to open postscript files in browser windows as well : [Attachment 6278] Patch v2

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Sun Feb 5 19:57:56 PST 2006


David D. Kilzer <ddkilzer at kilzer.net> has asked  for review:
Bug 3527: Allow Safari to open postscript files in browser windows as well
http://bugzilla.opendarwin.org/show_bug.cgi?id=3527

Attachment 6278: Patch v2
http://bugzilla.opendarwin.org/attachment.cgi?id=6278&action=edit

------- Additional Comments from David D. Kilzer <ddkilzer at kilzer.net>
Updates for this patch:

- Consolidated code changes to one class--WebPDFRepresentation.m.
- Added a number of ASSERT()s (too many?).
- Removed commented-out code.
- Changed 'NULL' to '0'.
- Added releases for converter, provider, consumer, data/result.
- Initialized the NSMutableData * data structure.

Regarding the use of NSMutableData * instead of CFMutableData, I found that the
following declaration would eventually cause a segmentation fault (after
viewing 3-6 PS files in the browser):

    CFMutableDataRef result = CFDataCreateMutable(kCFAllocatorDefault,
(CFIndex)[data length]);

if it replaced this call:

    NSMutableData *result = [[NSMutableData alloc] initWithCapacity:[data
length]];

Note that when I made the above change, I also changed how the CFMutableData
was released (using CFRelease()), but I still got the crash.

Finally, I'm not sure what should happen if CGPSConverterConvert() fails.  The
ASSERT(success) will throw some kind of assertion exception, I suppose.



More information about the webkit-reviews mailing list