[webkit-reviews] review denied: [Bug 177796] Break out new touch debugging code into seperate file : [Attachment 322483] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 3 01:12:32 PDT 2017


Simon Fraser (smfr) <simon.fraser at apple.com> has denied Megan Gardner
<megan_gardner at apple.com>'s request for review:
Bug 177796: Break out new touch debugging code into seperate file
https://bugs.webkit.org/show_bug.cgi?id=177796

Attachment 322483: Patch

https://bugs.webkit.org/attachment.cgi?id=322483&action=review




--- Comment #6 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 322483
  --> https://bugs.webkit.org/attachment.cgi?id=322483
Patch

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

> Tools/WebKitTestRunner/ios/HIDDebugTouchWindow.mm:41
> + at implementation HIDDebugTouchWindow

I think this would read better as GeneratedTouchesDebugWindow or something (the
HID isn't really interesting and may change).

> Tools/WebKitTestRunner/ios/HIDDebugTouchWindow.mm:61
> +- (instancetype)init
> +{
> +    self = [super init];
> +    if (!self)
> +	   return nil;
> +    
> +    return self;
> +}

This doesn't nothing; you can remove it.

> Tools/WebKitTestRunner/ios/HIDDebugTouchWindow.mm:89
> +	   UIApplicationRotationFollowingWindow *touchWindow =
[[UIApplicationRotationFollowingWindow alloc] init];

Leak, and you overwrite it 2 lines down.

> Tools/WebKitTestRunner/ios/HIDDebugTouchWindow.mm:90
> +	   UIApplicationRotationFollowingControllerNoTouches *viewController =
[[UIApplicationRotationFollowingControllerNoTouches alloc] init];

Leak

> Tools/WebKitTestRunner/ios/HIDDebugTouchWindow.mm:91
> +	   touchWindow = [[UIApplicationRotationFollowingWindow alloc] init];

Leak

> Tools/WebKitTestRunner/ios/HIDDebugTouchWindow.mm:105
> +	       [newView setBackgroundColor:[UIColor colorWithRed:1.0-i*.05
green:0.0 blue:1.0-i*.05 alpha:0.5]];

Spaces around operators please.


More information about the webkit-reviews mailing list