Changeset 259559 in webkit


Ignore:
Timestamp:
Apr 5, 2020 5:28:50 PM (4 years ago)
Author:
Wenson Hsieh
Message:

Address review comments after r259550
​https://bugs.webkit.org/show_bug.cgi?id=208472

Reviewed by Darin Adler.

  • platform/ios/ValidationBubbleIOS.mm:

Change a few named constants to be constexpr instead, and add comments describing their purpose.

(label):
(updateLabelFrame):

Rename these helper functions and move them up the file. Turn what was previously named
WebValidationBubbleViewController_labelFrame into updateLabelFrame, and have it additionally update the
view controller's label's frame to avoid repeating this logic in the subclassed method implementations below.

(callSuper):
(WebValidationBubbleViewController_viewDidLoad):
(WebValidationBubbleViewController_viewWillLayoutSubviews):
(WebValidationBubbleViewController_viewSafeAreaInsetsDidChange):
(allocWebValidationBubbleViewControllerInstance):

Instead of using -valueForKey, use objc_getAssociatedObject and objc_setAssociatedObject, with
OBJC_ASSOCIATION_RETAIN_NONATOMIC.

(WebCore::ValidationBubble::ValidationBubble):
(invokeUIViewControllerSelector): Deleted.
(WebValidationBubbleViewController_dealloc): Deleted.

Remove the -dealloc override. We don't need this anymore, since we're now using associated objects with
OBJC_ASSOCIATION_RETAIN_NONATOMIC to hold on to and keep track of our label.

(WebValidationBubbleViewController_labelFrame): Deleted.
(WebValidationBubbleViewController_label): Deleted.

Location:
trunk/Source/WebCore
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r259557 r259559  
     12020-04-05  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        Address review comments after r259550
     4        https://bugs.webkit.org/show_bug.cgi?id=208472
     5
     6        Reviewed by Darin Adler.
     7
     8        * platform/ios/ValidationBubbleIOS.mm:
     9
     10        Change a few named constants to be `constexpr` instead, and add comments describing their purpose.
     11
     12        (label):
     13        (updateLabelFrame):
     14
     15        Rename these helper functions and move them up the file. Turn what was previously named
     16        `WebValidationBubbleViewController_labelFrame` into `updateLabelFrame`, and have it additionally update the
     17        view controller's label's frame to avoid repeating this logic in the subclassed method implementations below.
     18
     19        (callSuper):
     20        (WebValidationBubbleViewController_viewDidLoad):
     21        (WebValidationBubbleViewController_viewWillLayoutSubviews):
     22        (WebValidationBubbleViewController_viewSafeAreaInsetsDidChange):
     23        (allocWebValidationBubbleViewControllerInstance):
     24
     25        Instead of using `-valueForKey`, use `objc_getAssociatedObject` and `objc_setAssociatedObject`, with
     26        `OBJC_ASSOCIATION_RETAIN_NONATOMIC`.
     27
     28        (WebCore::ValidationBubble::ValidationBubble):
     29        (invokeUIViewControllerSelector): Deleted.
     30        (WebValidationBubbleViewController_dealloc): Deleted.
     31
     32        Remove the -dealloc override. We don't need this anymore, since we're now using associated objects with
     33        `OBJC_ASSOCIATION_RETAIN_NONATOMIC` to hold on to and keep track of our label.
     34
     35        (WebValidationBubbleViewController_labelFrame): Deleted.
     36        (WebValidationBubbleViewController_label): Deleted.
     37
    1382020-04-05  Simon Fraser  <simon.fraser@apple.com>
    239

  • trunk/Source/WebCore/platform/ios/ValidationBubbleIOS.mm

    r259550 r259559  
    3232#import <UIKit/UIGeometry.h>
    3333#import <objc/message.h>
    34 #import <objc/runtime.h>
    3534#import <pal/ios/UIKitSoftLink.h>
    3635#import <pal/spi/ios/UIKitSPI.h>
    … …  
    3938#import <wtf/text/WTFString.h>
    4039
    41 static const CGFloat validationBubbleHorizontalPadding = 17;
    42 static const CGFloat validationBubbleVerticalPadding = 9;
    43 static const CGFloat validationBubbleMaxLabelWidth = 300;
     40// Add a bit of vertical and horizontal padding between the
     41// label and its parent view, to avoid laying out the label
     42// against the edges of the popover view.
     43constexpr CGFloat validationBubbleHorizontalPadding = 17;
     44constexpr CGFloat validationBubbleVerticalPadding = 9;
     45
     46// Avoid making the validation bubble too wide by enforcing a
     47// maximum width on the content size of the validation bubble
     48// view controller.
     49constexpr CGFloat validationBubbleMaxLabelWidth = 300;
     50
     51// Avoid making the validation bubble too tall by truncating
     52// the label to a maximum of 4 lines.
     53constexpr NSInteger validationBubbleMaxNumberOfLines = 4;
    4454
    4555@interface WebValidationBubbleViewController : UIViewController
    46 @property (nonatomic, readonly) UILabel *label;
    47 @property (nonatomic, readonly) CGRect labelFrame;
    48 @end
    49 
    50 static void invokeUIViewControllerSelector(id instance, SEL selector)
    51 {
    52     objc_super superClass { instance, PAL::getUIViewControllerClass() };
    53     auto superclassFunction = reinterpret_cast<void(*)(objc_super*, SEL)>(objc_msgSendSuper);
    54     superclassFunction(&superClass, selector);
    55 }
    56 
    57 static void WebValidationBubbleViewController_dealloc(WebValidationBubbleViewController *instance, SEL)
    58 {
    59     [instance.label release];
    60     [instance setValue:nil forKey:@"_label"];
    61 
    62     invokeUIViewControllerSelector(instance, @selector(dealloc));
     56@end
     57
     58static const void* const validationBubbleViewControllerLabelKey = &validationBubbleViewControllerLabelKey;
     59
     60static UILabel *label(WebValidationBubbleViewController *controller)
     61{
     62    return objc_getAssociatedObject(controller, validationBubbleViewControllerLabelKey);
     63}
     64
     65static void updateLabelFrame(WebValidationBubbleViewController *controller)
     66{
     67    auto frameWithPadding = UIEdgeInsetsInsetRect(controller.view.bounds, controller.view.safeAreaInsets);
     68    label(controller).frame = UIEdgeInsetsInsetRect(frameWithPadding, UIEdgeInsetsMake(validationBubbleVerticalPadding, validationBubbleHorizontalPadding, validationBubbleVerticalPadding, validationBubbleHorizontalPadding));
     69}
     70
     71static void callSuper(WebValidationBubbleViewController *instance, SEL selector)
     72{
     73    objc_super superStructure { instance, PAL::getUIViewControllerClass() };
     74    auto msgSendSuper = reinterpret_cast<void(*)(objc_super*, SEL)>(objc_msgSendSuper);
     75    msgSendSuper(&superStructure, selector);
    6376}
    6477
    6578static void WebValidationBubbleViewController_viewDidLoad(WebValidationBubbleViewController *instance, SEL)
    6679{
    67     invokeUIViewControllerSelector(instance, @selector(viewDidLoad));
    68 
    69     UILabel *label = [PAL::allocUILabelInstance() init];
    70     label.font = [PAL::getUIFontClass() preferredFontForTextStyle:PAL::get_UIKit_UIFontTextStyleCallout()];
    71     label.lineBreakMode = NSLineBreakByTruncatingTail;
    72     label.numberOfLines = 4;
    73     [instance.view addSubview:label];
    74     [instance setValue:label forKey:@"_label"];
     80    callSuper(instance, @selector(viewDidLoad));
     81
     82    auto label = adoptNS([PAL::allocUILabelInstance() init]);
     83    [label setFont:[PAL::getUIFontClass() preferredFontForTextStyle:PAL::get_UIKit_UIFontTextStyleCallout()]];
     84    [label setLineBreakMode:NSLineBreakByTruncatingTail];
     85    [label setNumberOfLines:validationBubbleMaxNumberOfLines];
     86    [instance.view addSubview:label.get()];
     87    objc_setAssociatedObject(instance, validationBubbleViewControllerLabelKey, label.get(), OBJC_ASSOCIATION_RETAIN_NONATOMIC);
    7588}
    7689
    7790static void WebValidationBubbleViewController_viewWillLayoutSubviews(WebValidationBubbleViewController *instance, SEL)
    7891{
    79     invokeUIViewControllerSelector(instance, @selector(viewWillLayoutSubviews));
    80 
    81     instance.label.frame = instance.labelFrame;
     92    callSuper(instance, @selector(viewWillLayoutSubviews));
     93    updateLabelFrame(instance);
    8294}
    8395
    8496static void WebValidationBubbleViewController_viewSafeAreaInsetsDidChange(WebValidationBubbleViewController *instance, SEL)
    8597{
    86     invokeUIViewControllerSelector(instance, @selector(viewSafeAreaInsetsDidChange));
    87 
    88     instance.label.frame = instance.labelFrame;
    89 }
    90 
    91 static CGRect WebValidationBubbleViewController_labelFrame(WebValidationBubbleViewController *instance, SEL)
    92 {
    93     auto frameWithPadding = UIEdgeInsetsInsetRect(instance.view.bounds, instance.view.safeAreaInsets);
    94     return UIEdgeInsetsInsetRect(frameWithPadding, UIEdgeInsetsMake(validationBubbleVerticalPadding, validationBubbleHorizontalPadding, validationBubbleVerticalPadding, validationBubbleHorizontalPadding));
    95 }
    96 
    97 static UILabel *WebValidationBubbleViewController_label(WebValidationBubbleViewController *instance, SEL)
    98 {
    99     return [instance valueForKey:@"_label"];
     98    callSuper(instance, @selector(viewSafeAreaInsetsDidChange));
     99    updateLabelFrame(instance);
    100100}
    101101
    … …  
    106106    dispatch_once(&onceToken, ^{
    107107        theClass = objc_allocateClassPair(PAL::getUIViewControllerClass(), "WebValidationBubbleViewController", 0);
    108         class_addMethod(theClass, @selector(dealloc), (IMP)WebValidationBubbleViewController_dealloc, "v@:");
    109108        class_addMethod(theClass, @selector(viewDidLoad), (IMP)WebValidationBubbleViewController_viewDidLoad, "v@:");
    110109        class_addMethod(theClass, @selector(viewWillLayoutSubviews), (IMP)WebValidationBubbleViewController_viewWillLayoutSubviews, "v@:");
    111110        class_addMethod(theClass, @selector(viewSafeAreaInsetsDidChange), (IMP)WebValidationBubbleViewController_viewSafeAreaInsetsDidChange, "v@:");
    112         class_addMethod(theClass, @selector(label), (IMP)WebValidationBubbleViewController_label, "v@:");
    113         class_addMethod(theClass, @selector(labelFrame), (IMP)WebValidationBubbleViewController_labelFrame, "v@:");
    114         class_addIvar(theClass, "_label", sizeof(UILabel *), log2(sizeof(UILabel *)), "@");
    115111        objc_registerClassPair(theClass);
    116112    });
    … …  
    178174    m_tapRecognizer = adoptNS([[WebValidationBubbleTapRecognizer alloc] initWithPopoverController:m_popoverController.get()]);
    179175
    180     UILabel *label = [m_popoverController label];
    181     label.text = message;
    182     m_fontSize = label.font.pointSize;
    183     CGSize labelSize = [label sizeThatFits:CGSizeMake(validationBubbleMaxLabelWidth, CGFLOAT_MAX)];
     176    UILabel *validationLabel = label(m_popoverController.get());
     177    validationLabel.text = message;
     178    m_fontSize = validationLabel.font.pointSize;
     179    CGSize labelSize = [validationLabel sizeThatFits:CGSizeMake(validationBubbleMaxLabelWidth, CGFLOAT_MAX)];
    184180    [m_popoverController setPreferredContentSize:CGSizeMake(labelSize.width + validationBubbleHorizontalPadding * 2, labelSize.height + validationBubbleVerticalPadding * 2)];
    185181}
Note: See TracChangeset for help on using the changeset viewer.