Bug 133582 – WK2 iOS: Adopt the new UIAlertController API to replace UIActionSheet

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Enrica Casucci]

UIActionSheet is deprecated in iOS8.

<rdar://problem/16845223>

[Enrica Casucci]

Created attachment 232626 [details]
Patch

[WebKit Commit Bot]

Attachment 232626 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm:200:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm:209:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm:210:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm:305:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm:307:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 5 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Enrica Casucci]

Wrong radar. This is the correct one. I fixed it in the ChangeLog <rdar://problem/16845223>

[Joseph Pecoraro]

Comment on attachment 232626 [details]
Patch

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

This looks good to me. A lot of small comments. But r=me

> Source/WebKit2/UIProcess/API/Cocoa/_WKElementAction.mm:50
> +    WKElementActionDismissalHandler _dismissalHandler;

I think this needs to be released in -dealloc because this is a (copy) property.

> Source/WebKit2/UIProcess/ios/WKActionSheet.h:34
> +@interface WKActionSheet : UIAlertController

Nit: We may want to rename this class and file. Fine for now.

> Source/WebKit2/UIProcess/ios/WKActionSheet.mm:44
> +    RetainPtr<id <UIPopoverPresentationControllerDelegate>> _popoverPresentationControllerDelegateWhileRotating;

Do you really want a RetainPtr<> here? This is a delegate.

Also, it looks like you can remove _delegateWhileRotating, it is no longer used.

> Source/WebKit2/UIProcess/ios/WKActionSheet.mm:95
> +    UIViewController *presentedViewController = (_presentedViewControllerWhileRotating.get()) ? _presentedViewControllerWhileRotating.get() : self;

Style: Unnecessary parenthesis around the ternary condition.

> Source/WebKit2/UIProcess/ios/WKActionSheet.mm:124
> +    // We want to save the view controller that is currently being presented to re-present it after rotation.

Typo: Extra "it" at the end of the sentence.

> Source/WebKit2/UIProcess/ios/WKActionSheet.mm:143
> +

Style: Extra blank line.

> Source/WebKit2/UIProcess/ios/WKActionSheet.mm:184
> +    UIViewController *presentedViewController = (_presentedViewControllerWhileRotating.get()) ? _presentedViewControllerWhileRotating.get() : self;

Style: Extra parenthesis.

> Source/WebKit2/UIProcess/ios/WKActionSheet.mm:196
>      _isRotating = NO;

Should "_isRotating = NO" be in -didRotate? Seems misleading that something else might change this internal only flag besides rotation.

> Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm:200
> +    for (_WKElementAction *action in actions)

Style: Give this for block braces

[Enrica Casucci]

Comment on attachment 232626 [details]
Patch

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

>> Source/WebKit2/UIProcess/API/Cocoa/_WKElementAction.mm:50
>> +    WKElementActionDismissalHandler _dismissalHandler;
> 
> I think this needs to be released in -dealloc because this is a (copy) property.

You are absolutely right. My bad.

>> Source/WebKit2/UIProcess/ios/WKActionSheet.mm:44
>> +    RetainPtr<id <UIPopoverPresentationControllerDelegate>> _popoverPresentationControllerDelegateWhileRotating;
> 
> Do you really want a RetainPtr<> here? This is a delegate.
> 
> Also, it looks like you can remove _delegateWhileRotating, it is no longer used.

The code that does the similar thing for WK1 calls retain when assigning _popoverPresentationControllerDelegateWhileRotating, that is why I opted for RetainPtr.

[Enrica Casucci]

Committed revision 169664.