Reversed WebState <-> CRWWebController ownership.

ios/web/web_state/ui/crw_web_controller.mm

Index: ios/web/web_state/ui/crw_web_controller.mm
diff --git a/ios/web/web_state/ui/crw_web_controller.mm b/ios/web/web_state/ui/crw_web_controller.mm
index 543697ea9b4f7e0721b6f8e01830d3a1da4cfdcb..cd4eaacd6966f63f1d8ca68a9ad3401ed896d82a 100644
--- a/ios/web/web_state/ui/crw_web_controller.mm
+++ b/ios/web/web_state/ui/crw_web_controller.mm
@@ -278,8 +278,8 @@ WKWebViewErrorSource WKWebViewErrorSourceFromError(NSError* error) {
 
   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.
@@ -597,12 +597,11 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
   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.
@@ -649,11 +648,11 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
 }
 
 - (WebState*)webState {
-  return _webStateImpl.get();
+  return _webStateImpl;
 }
 
 - (WebStateImpl*)webStateImpl {
-  return _webStateImpl.get();
+  return _webStateImpl;
 }
 
 - (void)clearTransientContentView {
@@ -896,9 +895,7 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
   // 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