Remove CWV class and move setting User Agent to CWVWebViewConfiguration.

ios/web_view/internal/cwv_web_view_configuration.mm

Index: ios/web_view/internal/cwv_web_view_configuration.mm
diff --git a/ios/web_view/internal/cwv_web_view_configuration.mm b/ios/web_view/internal/cwv_web_view_configuration.mm
index d8916d719da520424576cd898b40c9e11e064559..73ade0dc7833348ba90c69683b091207f177bc60 100644
--- a/ios/web_view/internal/cwv_web_view_configuration.mm
+++ b/ios/web_view/internal/cwv_web_view_configuration.mm
@@ -5,55 +5,78 @@
 #import "ios/web_view/public/cwv_web_view_configuration.h"
 #import "ios/web_view/internal/cwv_web_view_configuration_internal.h"
 
+#include "base/memory/ptr_util.h"
+#include "base/threading/thread_restrictions.h"
+#include "components/translate/core/browser/translate_download_manager.h"
+#include "ios/web/public/app/web_main.h"
 #import "ios/web_view/internal/cwv_user_content_controller_internal.h"
 #import "ios/web_view/internal/web_view_browser_state.h"
 #import "ios/web_view/internal/web_view_web_client.h"
+#import "ios/web_view/internal/web_view_web_main_delegate.h"
+#include "ui/base/l10n/l10n_util_mac.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
 
-@interface CWVWebViewConfiguration ()
+@interface CWVWebViewConfiguration () {
+  // The BrowserState for this configuration.
+  std::unique_ptr<ios_web_view::WebViewBrowserState> _browserState;
+}
+
 // Initialize configuration with the specified browser state.

[Eugene But (OOO till 7-30), 2017/04/11 17:52:42]

Would you mind fixing this comment s/Initialize/Initializes ?

[michaeldo, 2017/04/11 21:57:03]

Done.

-- (instancetype)initWithBrowserState:
-    (ios_web_view::WebViewBrowserState*)browserState;
+- (instancetype)initOffTheRecord:(BOOL)offTheRecord;

[Eugene But (OOO till 7-30), 2017/04/11 17:52:42]

I think that initializer should always start with "initWith" prefix and it's
quite hard to name this method. 
How about keeping the original signature?:

+ (instancetype)defaultConfiguration {
  web::BrowserState* browserState =
base::MakeUnique<ios_web_view::WebViewBrowserState>(false /*off_the_record*/);
  return [[self alloc] initWithBrowserState:std::move(browserState)];
}

[michaeldo, 2017/04/11 21:57:03]

I agree. Done.

 @end
 
-@implementation CWVWebViewConfiguration {
-  // TODO(crbug.com/690182): CWVWebViewConfiguration should own _browserState.
-  ios_web_view::WebViewBrowserState* _browserState;
-}
+// The web client associated with this configuration.
+static std::unique_ptr<ios_web_view::WebViewWebClient> webClient;

[Eugene But (OOO till 7-30), 2017/04/11 17:52:41]

Is there a reason for making this global? It is used only in one method and can
be a local variable, unless I'm missing something.

[michaeldo, 2017/04/11 21:57:02]

WebClient is assumed to be static in web::WebClient so we would need to relax
that assumption first if I understand it correctly.

https://cs.chromium.org/chromium/src/ios/web/public/web_client.h?rcl=4fad7069...

[Eugene But (OOO till 7-30), 2017/04/12 00:46:17]

My proposal was this:
+ (void)initialize {
  static std::unique_ptr<ios_web_view::WebViewWebClient> webClient;
  web::SetWebClient(&webClient);
}

|webClient| will be constructed when |initialize| is called and destructed at
application exit. webClient will remain the same so
|web::SetWebClient(&webClient);| call will be idempotent.

But you can just move |static std::unique_ptr<ios_web_view::WebViewWebClient>
webClient;| to initialize, unless I'm missing something.

[Eugene But (OOO till 7-30), 2017/04/12 17:48:27]

Please take a look at this comment

[michaeldo, 2017/04/12 18:22:55]

Sorry, yes you're correct. I forgot to move this after we stopped needing
webClient for the getter.

Done.

+
+@implementation CWVWebViewConfiguration
 
 @synthesize userContentController = _userContentController;
 
 + (instancetype)defaultConfiguration {
-  static dispatch_once_t once;
-  static CWVWebViewConfiguration* configuration;
-  dispatch_once(&once, ^{
-    ios_web_view::WebViewWebClient* client =
-        static_cast<ios_web_view::WebViewWebClient*>(web::GetWebClient());
-    configuration = [[self alloc] initWithBrowserState:client->browser_state()];
-  });
-  return configuration;
+  return [[CWVWebViewConfiguration alloc] initOffTheRecord:false];

[Eugene But (OOO till 7-30), 2017/04/11 17:52:41]

Do you want to keep old code ([self class]) to allow subclassing?

[michaeldo, 2017/04/11 21:57:03]

Done.

 }
 
 + (instancetype)incognitoConfiguration {
-  static dispatch_once_t once;
-  static CWVWebViewConfiguration* configuration;
-  dispatch_once(&once, ^{
-    ios_web_view::WebViewWebClient* client =
-        static_cast<ios_web_view::WebViewWebClient*>(web::GetWebClient());
-    configuration = [[self alloc]
-        initWithBrowserState:client->off_the_record_browser_state()];
-  });
-  return configuration;
+  return [[CWVWebViewConfiguration alloc] initOffTheRecord:true];
 }
 
-- (instancetype)initWithBrowserState:
-    (ios_web_view::WebViewBrowserState*)browserState {
+- (instancetype)initOffTheRecord:(BOOL)offTheRecord {
   self = [super init];
   if (self) {
-    _browserState = browserState;
+    static std::unique_ptr<ios_web_view::WebViewWebMainDelegate>

[Eugene But (OOO till 7-30), 2017/04/11 17:52:42]

Do you want to move globals initialization to +[CWVWebViewConfiguration
initialize]? This way we can add more designated initializers in the future and
will not clutter this init code.

[michaeldo, 2017/04/11 21:57:02]

Done.

+        webMainDelegate;
+    static std::unique_ptr<web::WebMain> webMain;
+    static dispatch_once_t onceToken;
+    dispatch_once(&onceToken, ^{
+      web::SetWebClient([[self class] webClient]);
+
+      webMainDelegate =
+          base::MakeUnique<ios_web_view::WebViewWebMainDelegate>();
+      web::WebMainParams params(webMainDelegate.get());
+      webMain = base::MakeUnique<web::WebMain>(params);
+    });
+
+    // IO access is required to setup the browser state. In Chrome, this is
+    // already allowed during thread startup. However, startup time of
+    // ChromeWebView is not predetermined, so IO access is temporarily allowed.
+    bool wasIOAllowed = base::ThreadRestrictions::SetIOAllowed(true);
+
+    _browserState =
+        base::MakeUnique<ios_web_view::WebViewBrowserState>(offTheRecord);
+
+    // Initialize translate.
+    translate::TranslateDownloadManager* downloadManager =
+        translate::TranslateDownloadManager::GetInstance();
+    downloadManager->set_request_context(_browserState->GetRequestContext());
+    // TODO(crbug.com/679895): Bring up application locale correctly.
+    downloadManager->set_application_locale(l10n_util::GetLocaleOverride());
+    downloadManager->language_list()->SetResourceRequestsAllowed(true);
+
+    base::ThreadRestrictions::SetIOAllowed(wasIOAllowed);
+
     _userContentController =
         [[CWVUserContentController alloc] initWithConfiguration:self];
   }
@@ -65,13 +88,19 @@
 }
 
 - (ios_web_view::WebViewBrowserState*)browserState {
-  return _browserState;
+  return _browserState.get();
+}
+
++ (ios_web_view::WebViewWebClient*)webClient {

[Eugene But (OOO till 7-30), 2017/04/11 17:52:41]

I don't feel that WebClient conceptually belongs to Configuration. Should this
be a separate C-function?

[michaeldo, 2017/04/11 21:57:03]

I agree it's odd here. I can actually delete this completely by storing the user
agent product as a static string inside CWVWebView and fetching it when needed
from WebViewWebClient. WDYT? (Please see the latest patch.)

+  webClient = base::MakeUnique<ios_web_view::WebViewWebClient>();

[Eugene But (OOO till 7-30), 2017/04/11 17:52:42]

This will recreate |webClient| every time when this method is called, which is
pretty unexpected side effect.

[michaeldo, 2017/04/11 21:57:03]

This was my mistake. Moved to static initialize method.

+  return webClient.get();
 }
 
 // NSCopying
 
 - (id)copyWithZone:(NSZone*)zone {
-  return [[[self class] allocWithZone:zone] initWithBrowserState:_browserState];
+  return [[[self class] allocWithZone:zone]
+      initOffTheRecord:_browserState->IsOffTheRecord()];
 }
 
 @end