Add CWVUserContentController which enables injecting JavaScripts.

ios/web_view/internal/cwv_user_content_controller.mm

+// Copyright 2017 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.

[michaeldo, 2017/03/21 16:12:34]

Please add new line after license.

[Hiroshi Ichikawa, 2017/03/22 04:52:52]

Done.

+#import "ios/web_view/public/cwv_user_content_controller.h"
+#import "ios/web_view/internal/cwv_user_content_controller_internal.h"
+
+#import "ios/web_view/internal/cwv_web_view_configuration_internal.h"
+#include "ios/web_view/internal/web_view_browser_state.h"
+#import "ios/web_view/internal/web_view_early_page_script_provider.h"
+#import "ios/web_view/public/cwv_user_script.h"
+
+@implementation CWVUserContentController {
+  __weak CWVWebViewConfiguration* _configuration;

[michaeldo, 2017/03/21 16:12:34]

Please use properties here

Ref:
https://google.github.io/styleguide/objcguide.xml?showone=Properties#Properties

[Hiroshi Ichikawa, 2017/03/22 04:52:52]

Done.

+  NSMutableArray<CWVUserScript*>* _userScripts;
+}
+
+@synthesize userScripts = _userScripts;
+
+- (instancetype)initWithConfiguration:
+    (__weak CWVWebViewConfiguration*)configuration {
+  self = [super init];
+  if (self) {
+    _configuration = configuration;

[Eugene But (OOO till 7-30), 2017/03/21 16:47:55]

nit: Consider adding DCHECK(configuration)

[Hiroshi Ichikawa, 2017/03/22 04:52:52]

Done. But I also added nonnull annotation for |configuration|, so maybe DCHECK
is not necessary? Are there cases where nil is passed to nonnull argument?

[michaeldo, 2017/03/22 23:03:32]

We commonly use DCHECKS where we expect a nonnull item, but that practice was
prior to annotating with nonnnull. I don't think the DCHECK is necessary with
the annotated method marked as nonnull, but I don't know if our team has any
specific guidance on this case. Nullability annotations are referenced from the
style guides at all from what I could see.

[Eugene But (OOO till 7-30), 2017/03/22 23:09:50]

That's a good point. Maybe DCHECKs are indeed unnecessary :)

[Hiroshi Ichikawa, 2017/03/23 02:48:05]

Thanks. Deleted the DCHECKs.

+    _userScripts = [[NSMutableArray alloc] init];
+  }
+  return self;
+}
+
+- (void)addUserScript:(CWVUserScript*)userScript {
+  [_userScripts addObject:userScript];

[Eugene But (OOO till 7-30), 2017/03/21 16:47:55]

nit: Consider adding DCHECK(userScript)

[Hiroshi Ichikawa, 2017/03/22 04:52:52]

Done.

+  [self updateEarlyPageScript];
+}
+
+- (void)removeAllUserScripts {
+  [_userScripts removeAllObjects];
+  [self updateEarlyPageScript];
+}
+
+- (void)updateEarlyPageScript {

[Eugene But (OOO till 7-30), 2017/03/21 16:47:55]

nit: Could you please add comment. 

[Hiroshi Ichikawa, 2017/03/22 04:52:52]

Done.

+  NSMutableString* joinedScript = [NSMutableString string];

[michaeldo, 2017/03/21 16:12:34]

please use alloc/init instead of "string"

[Hiroshi Ichikawa, 2017/03/22 04:52:52]

Done. Just curious, why? Is it just a preferred style?

[michaeldo, 2017/03/22 23:03:32]

Correct, I suggested this because our style prefers alloc/init over new and
string is a similar Class method shortcut initializer.

[Hiroshi Ichikawa, 2017/03/23 02:48:05]

Acknowledged.

+  for (CWVUserScript* script in _userScripts) {
+    [joinedScript appendString:script.source];

[Eugene But (OOO till 7-30), 2017/03/21 16:47:55]

This can potentially be a performance problem. Do you want to file a bug and add
TODO to refactor WebClient::GetEarlyPageScript so it returns
NSArray<NSString*>*?

[Hiroshi Ichikawa, 2017/03/22 04:52:52]

Done. I believe it would be rare that it becomes a performance problem because
this method is called just initially in most cases, though.

+    // Inserts "\n" between scripts to make it safer to join multiple scripts,
+    // in case the first script doesn't end with ";" or "\n".
+    [joinedScript appendString:@"\n"];
+  }
+  ios_web_view::WebViewEarlyPageScriptProvider::FromBrowserState(
+      _configuration.browserState)
+      .SetScript(joinedScript);
+}
+
+@end