[Webkit-unassigned] [Bug 146350] Add support for image previews

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jun 26 10:58:29 PDT 2015


https://bugs.webkit.org/show_bug.cgi?id=146350

--- Comment #3 from Tim Horton <thorton at apple.com> ---
Comment on attachment 255647
  --> https://bugs.webkit.org/attachment.cgi?id=255647
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=255647&action=review

> Source/WebKit2/UIProcess/WKImagePreviewViewController.h:32
> + at interface WKImagePreviewViewController : UIViewController {

no need for the curly braces

> Source/WebKit2/UIProcess/WKImagePreviewViewController.mm:53
> +    _imageView = [[UIImageView alloc] initWithFrame:CGRectZero];

This is leaking, no?

> Source/WebKit2/UIProcess/WKImagePreviewViewController.mm:54
> +    [_imageView setImage:[[UIImage alloc] initWithCGImage:_image.get()]];

This too, probably?

> Source/WebKit2/UIProcess/WKImagePreviewViewController.mm:56
> +    CGSize screenSize = [[[UIApplication sharedApplication] delegate] window].bounds.size;

More dots, less square brackets.

> Source/WebKit2/UIProcess/WKImagePreviewViewController.mm:70
> +    CGRect bounds = self.view.bounds;
> +    _imageView.frame = bounds;

No need for the temporary.

> Source/WebKit2/UIProcess/WKImagePreviewViewController.mm:73
> +static CGSize _scaleSizeWithinSize(CGSize source, CGSize destination)

Is this one of those GeometryUtilities (or whatever) things? If not, can it be?

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3146
> +        SFSafariViewController *previewViewController = [allocSFSafariViewControllerInstance() initWithURL:targetURL];

Seems likely that this leaks too?

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3157
> +        return [[WKImagePreviewViewController alloc] initWithCGImage:_positionInformation.image->makeCGImageCopy()];

And this?

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2142
> +                            RefPtr<ShareableBitmap> sharedBitmap = ShareableBitmap::createShareable(IntSize(image->size()), ShareableBitmap::SupportsAlpha);

Please always null-check the result of ShareableBitmap::ctreateShareable.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150626/607f002b/attachment-0001.html>


More information about the webkit-unassigned mailing list