Refactor creation of SadTabView into a tab helper object

ios/chrome/browser/ui/sad_tab/sad_tab_view.mm

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ios/chrome/browser/ui/sad_tab/sad_tab_view.h"
 
 #import "base/ios/weak_nsobject.h"
 #include "base/logging.h"
 #include "base/mac/scoped_block.h"
 #include "base/mac/scoped_nsobject.h"
@@ skipping 10 matching lines @@
 #import "ios/chrome/browser/ui/url_loader.h"
 #import "ios/chrome/browser/ui/util/label_link_controller.h"
 #import "ios/third_party/material_components_ios/src/components/Buttons/src/MaterialButtons.h"
 #import "ios/third_party/material_components_ios/src/components/Palettes/src/MaterialPalettes.h"
 #import "ios/third_party/material_roboto_font_loader_ios/src/src/MaterialRobotoFontLoader.h"
 #include "ios/web/public/interstitials/web_interstitial.h"
 #include "ios/web/public/web_state/web_state.h"
 #import "net/base/mac/url_conversions.h"
 #include "ui/base/l10n/l10n_util.h"
 
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
 namespace {
 // Color constants.
 const CGFloat kBackgroundColorBrightness = 247.0f / 255.0f;
 const CGFloat kTitleLabelTextColorBrightness = 22.0f / 255.0f;
 const CGFloat kMessageLabelTextColorBrightness = 80.0f / 255.0f;
 // Layout constants.
 const UIEdgeInsets kLayoutInsets = {24.0f, 24.0f, 24.0f, 24.0f};
 const CGFloat kLayoutBoundsMaxWidth = 600.0f;
 const CGFloat kContainerViewLandscapeTopPadding = 22.0f;
 const CGFloat kTitleLabelTopPadding = 26.0f;
 const CGFloat kMessageLabelTopPadding = 16.0f;
 const CGFloat kHelpLabelTopPadding = 16.0f;
 const CGFloat kReloadButtonHeight = 48.0f;
 const CGFloat kReloadButtonTopPadding = 16.0f;
 // Label font sizes.
 const CGFloat kTitleLabelFontSize = 23.0f;
 const CGFloat kMessageLabelFontSize = 14.0f;
 const CGFloat kHelpLabelFontSize = 14.0f;
 }  // namespace
 
-@interface SadTabView () {
-  // The block called when |_reloadButton| is tapped.
-  base::mac::ScopedBlock<ProceduralBlock> _reloadHandler;
-  // Backing objects for properties of the same name.
-  base::scoped_nsobject<UIView> _containerView;
-  base::scoped_nsobject<UIImageView> _imageView;
-  base::scoped_nsobject<UILabel> _titleLabel;
-  base::scoped_nsobject<UILabel> _messageLabel;
-  base::scoped_nsobject<UILabel> _helpLabel;
-  base::scoped_nsobject<LabelLinkController> _helpLabelLinkController;
-  base::scoped_nsobject<MDCButton> _reloadButton;
-}
+@interface SadTabView ()
 
+@property(nonatomic, copy) ProceduralBlock reloadHandler;
 // Container view that displays all other subviews.
-@property(nonatomic, readonly) UIView* containerView;
+@property(nonatomic, strong) UIView* containerView;
 // Displays the Sad Tab face.
-@property(nonatomic, readonly) UIImageView* imageView;
+@property(nonatomic, strong) UIImageView* imageView;
 // Displays the Sad Tab title.
-@property(nonatomic, readonly) UILabel* titleLabel;
+@property(nonatomic, strong) UILabel* titleLabel;
 // Displays the Sad Tab message.
-@property(nonatomic, readonly) UILabel* messageLabel;
+@property(nonatomic, strong) UILabel* messageLabel;
 // Displays the Sad Tab help message.
-@property(nonatomic, readonly) UILabel* helpLabel;
+@property(nonatomic, strong) UILabel* helpLabel;
+@property(nonatomic, strong) LabelLinkController* helpLabelLinkController;
 // Button used to trigger a reload.
-@property(nonatomic, readonly) UIButton* reloadButton;
+@property(nonatomic, strong) MDCFlatButton* reloadButton;
 
 // The bounds of |containerView|, with a height updated to CGFLOAT_MAX to allow
 // text to be laid out using as many lines as necessary.
 @property(nonatomic, readonly) CGRect containerBounds;
 
 // Subview layout methods.  Must be called in the following order, as subsequent
 // layouts reference the values set in previous functions.
 - (void)layoutImageView;
 - (void)layoutTitleLabel;
 - (void)layoutMessageLabel;
 - (void)layoutHelpLabel;
 - (void)layoutReloadButton;
 - (void)layoutContainerView;
 
 // The action selector for |_reloadButton|.
 - (void)handleReloadButtonTapped;
 
 // Returns the desired background color.
 + (UIColor*)sadTabBackgroundColor;
 
 @end
 
 #pragma mark - SadTabView
 
 @implementation SadTabView
 
+@synthesize imageView = _imageView;
+@synthesize containerView = _containerView;
+@synthesize titleLabel = _titleLabel;
+@synthesize messageLabel = _messageLabel;
+@synthesize helpLabel = _helpLabel;
+@synthesize helpLabelLinkController = _helpLabelLinkController;
+@synthesize reloadButton = _reloadButton;
+@synthesize reloadHandler = _reloadHandler;
+
 - (instancetype)initWithReloadHandler:(ProceduralBlock)reloadHandler {
   self = [super initWithFrame:CGRectZero];
   if (self) {
     DCHECK(reloadHandler);
-    _reloadHandler.reset([reloadHandler copy]);
+    _reloadHandler = reloadHandler;
     self.backgroundColor = [[self class] sadTabBackgroundColor];
   }
   return self;
 }
 
 - (instancetype)init {
   NOTREACHED();
   return nil;
 }
 
 - (instancetype)initWithFrame:(CGRect)frame {
   NOTREACHED();
   return nil;
 }
 
 - (instancetype)initWithCoder:(NSCoder*)aDecoder {
   NOTREACHED();
   return nil;
 }
 
 #pragma mark Accessors
 
 - (UIView*)containerView {
   if (!_containerView) {
-    _containerView.reset([[UIView alloc] initWithFrame:CGRectZero]);
+    _containerView = [[UIView alloc] initWithFrame:CGRectZero];
     [_containerView setBackgroundColor:self.backgroundColor];
   }
   return _containerView;
 }
 
 - (UIImageView*)imageView {
   if (!_imageView) {
-    _imageView.reset(
-        [[UIImageView alloc] initWithImage:NativeImage(IDR_CRASH_SAD_TAB)]);
+    _imageView =
+        [[UIImageView alloc] initWithImage:NativeImage(IDR_CRASH_SAD_TAB)];
     [_imageView setBackgroundColor:self.backgroundColor];
   }
-  return _imageView.get();
+  return _imageView;
 }
 
 - (UILabel*)titleLabel {
   if (!_titleLabel) {
-    _titleLabel.reset([[UILabel alloc] initWithFrame:CGRectZero]);
+    _titleLabel = [[UILabel alloc] initWithFrame:CGRectZero];
     [_titleLabel setBackgroundColor:self.backgroundColor];
     [_titleLabel setText:base::SysUTF8ToNSString(
                              l10n_util::GetStringUTF8(IDS_SAD_TAB_TITLE))];
     [_titleLabel setLineBreakMode:NSLineBreakByWordWrapping];
     [_titleLabel setNumberOfLines:0];
     [_titleLabel
         setTextColor:[UIColor colorWithWhite:kTitleLabelTextColorBrightness
                                        alpha:1.0]];
     [_titleLabel setFont:[[MDFRobotoFontLoader sharedInstance]
                              regularFontOfSize:kTitleLabelFontSize]];
   }
-  return _titleLabel.get();
+  return _titleLabel;
 }
 
 - (UILabel*)messageLabel {
   if (!_messageLabel) {
-    _messageLabel.reset([[UILabel alloc] initWithFrame:CGRectZero]);
+    _messageLabel = [[UILabel alloc] initWithFrame:CGRectZero];
     [_messageLabel setBackgroundColor:self.backgroundColor];
     std::string messageText = l10n_util::GetStringUTF8(IDS_SAD_TAB_MESSAGE);
     [_messageLabel setText:base::SysUTF8ToNSString(messageText)];
     [_messageLabel setLineBreakMode:NSLineBreakByWordWrapping];
     [_messageLabel setNumberOfLines:0];
     [_messageLabel
         setTextColor:[UIColor colorWithWhite:kMessageLabelTextColorBrightness
                                        alpha:1.0]];
     [_messageLabel setFont:[[MDFRobotoFontLoader sharedInstance]
                                regularFontOfSize:kMessageLabelFontSize]];
   }
-  return _messageLabel.get();
+  return _messageLabel;
 }
 
 - (UILabel*)helpLabel {
   if (!_helpLabel) {
-    _helpLabel.reset([[UILabel alloc] initWithFrame:CGRectZero]);
+    _helpLabel = [[UILabel alloc] initWithFrame:CGRectZero];
     [_helpLabel setBackgroundColor:self.backgroundColor];
     [_helpLabel setNumberOfLines:0];
     [_helpLabel setFont:[[MDFRobotoFontLoader sharedInstance]
                             regularFontOfSize:kHelpLabelFontSize]];
     [_helpLabel
         setTextColor:[UIColor colorWithWhite:kMessageLabelTextColorBrightness
                                        alpha:1.0]];
     // Fetch help text.
     base::string16 helpLinkText(
         l10n_util::GetStringUTF16(IDS_SAD_TAB_HELP_LINK));
     NSString* helpText = base::SysUTF16ToNSString(
         l10n_util::GetStringFUTF16(IDS_SAD_TAB_HELP_MESSAGE, helpLinkText));
     [_helpLabel setText:helpText];
     // Create link controller.
     base::WeakNSObject<SadTabView> weakSelf(self);
-    _helpLabelLinkController.reset([[LabelLinkController alloc]
+    _helpLabelLinkController = [[LabelLinkController alloc]
         initWithLabel:_helpLabel
                action:^(const GURL& url) {
                  base::scoped_nsobject<OpenUrlCommand> openCommand(
                      [[OpenUrlCommand alloc] initWithURLFromChrome:url]);
                  [weakSelf chromeExecuteCommand:openCommand];
-               }]);
+               }];
     [_helpLabelLinkController
         setLinkFont:[[MDFRobotoFontLoader sharedInstance]
                         boldFontOfSize:kHelpLabelFontSize]];
     [_helpLabelLinkController setLinkUnderlineStyle:NSUnderlineStyleSingle];
     NSRange linkRange =
         [helpText rangeOfString:base::SysUTF16ToNSString(helpLinkText)];
     DCHECK_NE(linkRange.location, static_cast<NSUInteger>(NSNotFound));
     DCHECK_NE(linkRange.length, 0U);
     [_helpLabelLinkController addLinkWithRange:linkRange
                                            url:GURL(kCrashReasonURL)];
   }
-  return _helpLabel.get();
+  return _helpLabel;
 }
 
 - (UIButton*)reloadButton {
   if (!_reloadButton) {
-    _reloadButton.reset([[MDCFlatButton alloc] init]);
+    _reloadButton = [[MDCFlatButton alloc] init];
     [_reloadButton setBackgroundColor:[[MDCPalette cr_bluePalette] tint500]
                              forState:UIControlStateNormal];
     [_reloadButton setBackgroundColor:[[MDCPalette greyPalette] tint500]
                              forState:UIControlStateDisabled];
     [_reloadButton setCustomTitleColor:[UIColor whiteColor]];
     [_reloadButton setUnderlyingColorHint:[UIColor blackColor]];
     [_reloadButton setInkColor:[UIColor colorWithWhite:1 alpha:0.2f]];
     NSString* title = base::SysUTF8ToNSString(
         l10n_util::GetStringUTF8(IDS_SAD_TAB_RELOAD_LABEL));
     [_reloadButton setTitle:title forState:UIControlStateNormal];
     [_reloadButton setTitleColor:[UIColor whiteColor]
                         forState:UIControlStateNormal];
     [_reloadButton addTarget:self
                       action:@selector(handleReloadButtonTapped)
             forControlEvents:UIControlEventTouchUpInside];
   }
-  return _reloadButton.get();
+  return _reloadButton;
 }
 
 - (CGRect)containerBounds {
   CGFloat containerWidth = std::min(
       CGRectGetWidth(self.bounds) - kLayoutInsets.left - kLayoutInsets.right,
       kLayoutBoundsMaxWidth);
   return CGRectMake(0.0, 0.0, containerWidth, CGFLOAT_MAX);
 }
 
 #pragma mark Layout
@@ skipping 131 matching lines @@
 
 #pragma mark Util
 
 - (void)handleHelpLabelLinkButtonTapped {
   base::scoped_nsobject<OpenUrlCommand> openCommand(
       [[OpenUrlCommand alloc] initWithURLFromChrome:GURL(kCrashReasonURL)]);
   [self chromeExecuteCommand:openCommand];
 }
 
 - (void)handleReloadButtonTapped {
-  _reloadHandler.get()();
+  if (self.reloadHandler) {

[kkhorimoto, 2017/04/10 22:31:09]

We're already DCHECKing that the reload handler is non-nil in the initializer,
so this extra check isn't needed.

[PL, 2017/04/11 01:35:21]

Happy to take this check out if you feel strongly here, but here's my thinking:

• DCHECK (I think) only blows up if we're in Debug, but we'll always blow up
with a nil handler in shipping software so I think we should be wary of
considering that DCHECK will catch everything here?

• It's not possible to guarantee that this class won't change subtly in the 
future where the handler will be nullable etc. so I'd rather take the precaution
now. 

What do you think?

[kkhorimoto, 2017/04/11 02:13:15]

The Chromium style guide says that if we're DCHECKing something, we should not
add code that handles when the DCHECK fails:
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

That being said, you're right that DCHECKing in the initializer might not be
sufficient if the implementation of this class changes and the reload handler is
nil'd out somehow.  However, that is not a state we ever want to get this class
into (i.e. a reload button that doesn't do anything), so how about adding an
additional DCHECK here in this function, then calling |reloadHandler|
immediately afterward. 

[PL, 2017/04/11 03:08:56]

Done! I'm a little nervous but I trust your guidance!

+    self.reloadHandler();
+  }
 }
 
 + (UIColor*)sadTabBackgroundColor {
   return [UIColor colorWithWhite:kBackgroundColorBrightness alpha:1.0];
 }
 
 @end