Reversed WebState <-> CRWWebController ownership.

ios/web/web_state/ui/crw_web_controller.mm

 // Copyright 2012 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.
 
 #import "ios/web/web_state/ui/crw_web_controller.h"
 
 #import <objc/runtime.h>
 #include <stddef.h>
 
 #include <cmath>
@@ skipping 260 matching lines @@
   // reconstruction that should use cache aggressively.
   GURL _expectedReconstructionURL;
   // Whether the web page is currently performing window.history.pushState or
   // window.history.replaceState
   // Set to YES on window.history.willChangeState message. To NO on
   // window.history.didPushState or window.history.didReplaceState.
   BOOL _changingHistoryState;
 
   std::unique_ptr<web::NewWindowInfo> _externalRequest;
 
-  // The WebStateImpl instance associated with this CRWWebController.
-  std::unique_ptr<WebStateImpl> _webStateImpl;
+  // Unowned WebStateImpl instance associated with this CRWWebController.

[kkhorimoto, 2016/04/12 21:36:13]

"Unowned" seems kinda strangely worded.  It would be clearer if you instead
mentioned that |_webStateImpl| owns |self|.  Also, do you think it'd be worth it
to have a WeakPtrFactor owned by WebStateImpl so we can mitigate the possibility
of classes holding onto pointers to deleted objects?

[Eugene But (OOO till 7-30), 2016/04/13 02:51:26]

WC should not make any assumptions about it's owner. So I don't want to write
that WC is owned by WS. However I'm very open for replacing "Unowned" with
something else, so let me know if you have suggestions.

As for WeakFactory, I don't think it make sense to pay performance cost here. WC
clears reference to WS inside close and if WS had died before |close|, I think
we have a bigger problem than crashes :)

[Eugene But (OOO till 7-30), 2016/04/13 13:57:58]

Eliminated "unowned" word.

+  WebStateImpl* _webStateImpl;
 
   // A set of URLs opened in external applications; stored so that errors
   // from the web view can be identified as resulting from these events.
   base::scoped_nsobject<NSMutableSet> _openedApplicationURL;
 
   // Object that manages all early script injection into the web view.
   base::scoped_nsobject<CRWJSEarlyScriptManager> _earlyScriptManager;
 
   // Script manager for setting the windowID.
   base::scoped_nsobject<CRWJSWindowIdManager> _windowIDJSManager;
@@ skipping 297 matching lines @@
 + (instancetype)allocWithZone:(struct _NSZone*)zone {
   if (self == [CRWWebController class]) {
     // This is an abstract class which should not be instantiated directly.
     // Callers should create concrete subclasses instead.
     NOTREACHED();
     return nil;
   }
   return [super allocWithZone:zone];
 }
 
-- (instancetype)initWithWebState:(std::unique_ptr<WebStateImpl>)webState {
+- (instancetype)initWithWebState:(WebStateImpl*)webState {
   self = [super init];
   if (self) {
-    _webStateImpl = std::move(webState);
+    _webStateImpl = webState;
     DCHECK(_webStateImpl);
-    _webStateImpl->SetWebController(self);
     _webStateImpl->InitializeRequestTracker(self);
     // Load phase when no WebView present is 'loaded' because this represents
     // the idle state.
     _loadPhase = web::PAGE_LOADED;
     // Content area is lazily instantiated.
     _defaultURL = GURL(url::kAboutBlankURL);
     _jsInjectionReceiver.reset(
         [[CRWJSInjectionReceiver alloc] initWithEvaluator:self]);
     _earlyScriptManager.reset([(CRWJSEarlyScriptManager*)[_jsInjectionReceiver
         instanceOfClass:[CRWJSEarlyScriptManager class]] retain]);
@@ skipping 26 matching lines @@
 - (id<CRWSwipeRecognizerProvider>)swipeRecognizerProvider {
   return _swipeRecognizerProvider.get();
 }
 
 - (void)setSwipeRecognizerProvider:
     (id<CRWSwipeRecognizerProvider>)swipeRecognizerProvider {
   _swipeRecognizerProvider.reset(swipeRecognizerProvider);
 }
 
 - (WebState*)webState {
-  return _webStateImpl.get();
+  return _webStateImpl;
 }
 
 - (WebStateImpl*)webStateImpl {
-  return _webStateImpl.get();
+  return _webStateImpl;
 }
 
 - (void)clearTransientContentView {
   // Early return if there is no transient content view.
   if (!self.containerView.transientContentView)
     return;
 
   // Remove the transient content view from the hierarchy.
   [self.containerView clearTransientContentView];
 
@@ skipping 222 matching lines @@
 
   DCHECK(!_isBeingDestroyed);
   DCHECK(!_delegate);  // Delegate should reset its association before closing.
   // Mark the destruction sequence has started, in case someone else holds a
   // strong reference and tries to continue using the tab.
   _isBeingDestroyed = YES;
 
   // Remove the web view now. Otherwise, delegate callbacks occur.
   [self removeWebViewAllowingCachedReconstruction:NO];
 
-  // Tear down web ui (in case this is part of this tab) and web state now,
-  // since the timing of dealloc can't be guaranteed.
-  _webStateImpl.reset();
+  _webStateImpl = nullptr;
 }
 
 - (void)checkLinkPresenceUnderGesture:(UIGestureRecognizer*)gestureRecognizer
                     completionHandler:(void (^)(BOOL))completionHandler {
   CGPoint webViewPoint = [gestureRecognizer locationInView:self.webView];
   base::WeakNSObject<CRWWebController> weakSelf(self);
   [self
       fetchDOMElementAtPoint:webViewPoint
            completionHandler:^(std::unique_ptr<base::DictionaryValue> element) {
              std::string link;
@@ skipping 3165 matching lines @@
   }
 
   return web::WEB_VIEW_DOCUMENT_TYPE_GENERIC;
 }
 
 - (NSString*)refererFromNavigationAction:(WKNavigationAction*)action {
   return [action.request valueForHTTPHeaderField:@"Referer"];
 }
 
 @end