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

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

Index: chrome/browser/ui/cocoa/tab_contents/sad_tab_controller.mm
diff --git a/chrome/browser/ui/cocoa/tab_contents/sad_tab_controller.mm b/chrome/browser/ui/cocoa/tab_contents/sad_tab_controller.mm
index 747dfe23d8d0c9698334b120141cab037e39c88b..fd0437742ca3c0ffd7674b887c44e3ae94b69bb3 100644
--- a/chrome/browser/ui/cocoa/tab_contents/sad_tab_controller.mm
+++ b/chrome/browser/ui/cocoa/tab_contents/sad_tab_controller.mm
@@ -2,11 +2,12 @@
 // 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"
-
-#include "base/mac/bundle_locations.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 "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;
@@ -36,7 +37,13 @@ void SadTabCocoa::Close() {
 
 }  // 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])) {
@@ -56,27 +63,34 @@ void SadTabCocoa::Close() {
 }
 
 - (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);
 }