[Mac] Clean up SadTabView and let SadTabController provide its content

chrome/browser/ui/cocoa/tab_contents/sad_tab_controller.mm

 // Copyright (c) 2011 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.
 
-#include "chrome/browser/ui/cocoa/tab_contents/sad_tab_controller.h"
+#import "chrome/browser/ui/cocoa/tab_contents/sad_tab_controller.h"
+#import "chrome/browser/ui/cocoa/tab_contents/sad_tab_view_cocoa.h"

[Avi (use Gerrit), 2016/08/12 23:06:22]

Put this down with the other includes. The reason sad_tab_controller.h is alone
is because it's the corresponding .h file for this .mm. Other .h files live in
the main include block.

[Sidney San Martín, 2016/08/12 23:35:10]

Ah, that makes sense. Done.

 
-#include "base/mac/bundle_locations.h"
-#import "chrome/browser/ui/cocoa/tab_contents/sad_tab_view_cocoa.h"
 #include "chrome/common/url_constants.h"
+#include "chrome/grit/generated_resources.h"
+#include "components/strings/grit/components_strings.h"
 #include "content/public/browser/web_contents.h"
 
 using content::OpenURLParams;
 using content::Referrer;
 
 namespace chrome {
 
 SadTab* SadTab::Create(content::WebContents* web_contents, SadTabKind kind) {
   return new SadTabCocoa(web_contents);
 }
 
 SadTabCocoa::SadTabCocoa(content::WebContents* web_contents)
     : web_contents_(web_contents) {
 }
 
 SadTabCocoa::~SadTabCocoa() {
 }
 
 void SadTabCocoa::Show() {
   sad_tab_controller_.reset(
       [[SadTabController alloc] initWithWebContents:web_contents_]);
 }
 
 void SadTabCocoa::Close() {
   [[sad_tab_controller_ view] removeFromSuperview];
 }
 
 }  // namespace chrome
 
-@implementation SadTabController
+@interface SadTabController ()<SadTabViewDelegate>
+@end
+
+@implementation SadTabController {
+  content::WebContents* webContents_;
+  SadTabView* sadTabView_;
+}

[Avi (use Gerrit), 2016/08/12 23:06:22]

Doesn't this block go on the interface with a class extension?

[Sidney San Martín, 2016/08/12 23:35:10]

This is the closest I can find to a comment on it:
http://stackoverflow.com/a/10407891/84745

It includes this excerpt from The Objective-C Programming Language:
So I think it's recommended by Apple (and meant to be thought of like
@synthesize, which also declares ivars in the @implementation section), but I'm
not sure if we have a stance on it. 

[Avi (use Gerrit), 2016/08/12 23:43:16]

I'm working from
https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/Progra...
which in the section "Class Extensions Extend the Internal Implementation" has:

It’s also possible to use a class extension to add custom instance variables.
These are declared inside braces in the class extension interface:

@interface XYZPerson () {
    id _someCustomInstanceVariable;
}
...
@end

[Sidney San Martín, 2016/08/14 00:34:32]

Turns out the doc is inconsistent. Another section,
https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/Progra...,
says:
I favor the implementation over a class extension because:

- @synthesize declares instance variables for properties and can only appear in
the implementation. Having other instance variables there keeps them
together[1].

- Instance variables end up closer to the code that uses them.

- When the class doesn't otherwise need an extension, you don't have to write
one.

[1]: From what I can tell, support for declaring ivars in the implementation was
added to ObjC in the same pass as properties.

[Avi (use Gerrit), 2016/08/14 04:54:28]

Hmmm. Go for it, then.

 
 - (id)initWithWebContents:(content::WebContents*)webContents {
   if ((self = [super init])) {
     DCHECK(webContents);
     webContents_ = webContents;
 
     if (webContents_) {  // NULL in unit_tests.
       NSView* ns_view = webContents_->GetNativeView();
       [[self view] setAutoresizingMask:
           (NSViewWidthSizable | NSViewHeightSizable)];
       [ns_view addSubview:[self view]];
       [[self view] setFrame:[ns_view bounds]];
     }
   }
 
   return self;
 }
 
 - (void)dealloc {
-  [[sadTabView_ reloadButton] setTarget:nil];
+  sadTabView_.delegate = nil;
   [super dealloc];
 }
 
 - (void)loadView {
-  sadTabView_.reset([[SadTabView alloc] init]);
-  [[sadTabView_ reloadButton] setTarget:self];
-  [[sadTabView_ reloadButton] setAction:@selector(reloadPage:)];
-  [self setView:sadTabView_];
+  sadTabView_ = [[SadTabView new] autorelease];
+  sadTabView_.delegate = self;
+
+  [sadTabView_ setTitle:IDS_SAD_TAB_TITLE];
+  [sadTabView_ setMessage:IDS_SAD_TAB_MESSAGE];
+  [sadTabView_ setButtonTitle:IDS_SAD_TAB_RELOAD_LABEL];
+  [sadTabView_ setHelpLinkTitle:IDS_SAD_TAB_LEARN_MORE_LINK
+                            URL:@(chrome::kCrashReasonURL)];
+
+  self.view = sadTabView_;
 }
 
 - (content::WebContents*)webContents {
   return webContents_;
 }
 
-- (IBAction)reloadPage:(id)sender {
+- (void)sadTabViewButtonClicked:(SadTabView*)sadTabView {
   webContents_->GetController().Reload(true);
 }
 
-- (void)openLearnMoreAboutCrashLink:(id)sender {
-  OpenURLParams params(GURL(chrome::kCrashReasonURL), Referrer(), CURRENT_TAB,
+- (void)sadTabView:(SadTabView*)sadTabView
+    helpLinkClickedWithURL:(NSString*)url {
+  OpenURLParams params(GURL(url.UTF8String), Referrer(), CURRENT_TAB,
                        ui::PAGE_TRANSITION_LINK, false);
   webContents_->OpenURL(params);
 }
 
 @end