Error pages: Break retain cycle NativeContent/StaticHTMLViewController

ios/chrome/browser/web/error_page_content.mm

Index: ios/chrome/browser/web/error_page_content.mm
diff --git a/ios/chrome/browser/web/error_page_content.mm b/ios/chrome/browser/web/error_page_content.mm
index fbd846d316c1c82a740d820beddd0463048d2cf7..71b03005fa4bad304821665b6a13459315840144 100644
--- a/ios/chrome/browser/web/error_page_content.mm
+++ b/ios/chrome/browser/web/error_page_content.mm
@@ -7,6 +7,7 @@
 #include <memory>
 
 #import "base/ios/ns_error_util.h"
+#include "base/mac/scoped_nsobject.h"

[Eugene But (OOO till 7-30), 2017/02/17 18:37:54]

nit: s/include/import

[Olivier, 2017/02/20 08:47:00]

Done.

 #include "base/strings/sys_string_conversions.h"
 #include "base/values.h"
 #include "components/error_page/common/error_page_params.h"
@@ -20,6 +21,71 @@
 #include "ui/base/webui/jstemplate_builder.h"
 #include "url/gurl.h"
 
+#pragma mark - ErrorPageGenerator
+
+@interface ErrorPageGenerator : NSObject<HtmlGenerator>

[Eugene But (OOO till 7-30), 2017/02/17 18:37:54]

Please add comments to class and initializer. F.e. it is useful to explain that
ErrorPageGenerator is needed because StaticHtmlViewController retains generator
argument. 

[Olivier, 2017/02/20 08:47:00]

Done.

+- (instancetype)initWithError:(NSError*)error
+                       isPost:(BOOL)isPost
+                  isIncognito:(BOOL)isIncognito NS_DESIGNATED_INITIALIZER;
+
+- (instancetype)init NS_UNAVAILABLE;
+@end
+
+@implementation ErrorPageGenerator {
+  // Stores the HTML generated from the NSError in the initializer.
+  base::scoped_nsobject<NSString> html_;
+}
+
+- (instancetype)initWithError:(NSError*)error
+                       isPost:(BOOL)isPost
+                  isIncognito:(BOOL)isIncognito {
+  self = [super init];
+  if (self) {
+    NSString* badURLString =

[Eugene But (OOO till 7-30), 2017/02/17 18:37:54]

Optional nit. This would fit 80 symbols:

NSString* badURLSpec = error.userInfo[NSURLErrorFailingURLStringErrorKey];

[Olivier, 2017/02/20 08:47:00]

Done.

+        [[error userInfo] objectForKey:NSURLErrorFailingURLStringErrorKey];
+    NSError* originalError = base::ios::GetFinalUnderlyingErrorFromError(error);
+    NSString* errorDomain = nil;
+    int errorCode = 0;
+
+    if (originalError) {
+      errorDomain = [originalError domain];
+      errorCode = [originalError code];
+    } else {
+      errorDomain = [error domain];
+      errorCode = [error code];
+    }
+    DCHECK(errorCode != 0);
+
+    base::DictionaryValue errorStrings;
+    error_page::LocalizedError::GetStrings(
+        errorCode, base::SysNSStringToUTF8(errorDomain),
+        GURL(base::SysNSStringToUTF16(badURLString)), isPost, false, false,
+        isIncognito, GetApplicationContext()->GetApplicationLocale(), nullptr,
+        &errorStrings);
+
+    ui::ScaleFactor scaleFactor =
+        ResourceBundle::GetSharedInstance().GetMaxScaleFactor();
+
+    const base::StringPiece templateHTML(
+        ResourceBundle::GetSharedInstance().GetRawDataResourceForScale(
+            IDR_NET_ERROR_HTML, scaleFactor));
+    if (templateHTML.empty())
+      NOTREACHED() << "unable to load template. ID: " << IDR_NET_ERROR_HTML;
+    std::string errorHTML = webui::GetTemplatesHtml(
+        templateHTML, &errorStrings, "t" /* IDR_NET_ERROR_HTML root id */);
+    html_.reset([base::SysUTF8ToNSString(errorHTML) retain]);
+  }
+  return self;
+}
+

[Eugene But (OOO till 7-30), 2017/02/17 18:37:54]

Optional nit: pragma mark HtmlGenerator

[Olivier, 2017/02/20 08:47:00]

Done.

+- (void)generateHtml:(HtmlCallback)callback {
+  callback(html_.get());
+}
+
+@end
+
+#pragma mark - ErrorPageContent
+
 @implementation ErrorPageContent
 
 #pragma mark -
@@ -36,52 +102,19 @@
       rendererInitiated:YES];
 }
 
-- (void)generateHtml:(HtmlCallback)callback {
-  callback(html_.get());
-}
-
 - (id)initWithLoader:(id<UrlLoader>)loader
         browserState:(web::BrowserState*)browserState
                  url:(const GURL&)url
                error:(NSError*)error
               isPost:(BOOL)isPost
          isIncognito:(BOOL)isIncognito {
-  NSString* badURLString =
-      [[error userInfo] objectForKey:NSURLErrorFailingURLStringErrorKey];
-  NSError* originalError = base::ios::GetFinalUnderlyingErrorFromError(error);
-  NSString* errorDomain = nil;
-  int errorCode = 0;
-
-  if (originalError) {
-    errorDomain = [originalError domain];
-    errorCode = [originalError code];
-  } else {
-    errorDomain = [error domain];
-    errorCode = [error code];
-  }
-  DCHECK(errorCode != 0);
-
-  base::DictionaryValue errorStrings;
-  error_page::LocalizedError::GetStrings(
-      errorCode, base::SysNSStringToUTF8(errorDomain),
-      GURL(base::SysNSStringToUTF16(badURLString)), isPost, false, false,
-      isIncognito, GetApplicationContext()->GetApplicationLocale(), nullptr,
-      &errorStrings);
-
-  ui::ScaleFactor scaleFactor =
-      ResourceBundle::GetSharedInstance().GetMaxScaleFactor();
-
-  const base::StringPiece templateHTML(
-      ResourceBundle::GetSharedInstance().GetRawDataResourceForScale(
-          IDR_NET_ERROR_HTML, scaleFactor));
-  if (templateHTML.empty())
-    NOTREACHED() << "unable to load template. ID: " << IDR_NET_ERROR_HTML;
-  std::string errorHTML = webui::GetTemplatesHtml(
-      templateHTML, &errorStrings, "t" /* IDR_NET_ERROR_HTML root id */);
-  html_.reset([base::SysUTF8ToNSString(errorHTML) retain]);
+  ErrorPageGenerator* generator =
+      [[[ErrorPageGenerator alloc] initWithError:error
+                                          isPost:isPost
+                                     isIncognito:isIncognito] autorelease];
 
   base::scoped_nsobject<StaticHtmlViewController> HTMLViewController(
-      [[StaticHtmlViewController alloc] initWithGenerator:self
+      [[StaticHtmlViewController alloc] initWithGenerator:generator
                                              browserState:browserState]);
   return [super initWithLoader:loader
       staticHTMLViewController:HTMLViewController