Clean shut down for //ios/web_view.

If you quit the app in iOS multitasking, it DCHECKs in a few places. This makes it impossible to
write integration tests.

This happens because, in our current setup, ObjC classes stay around longer than C++ classes.
More specifically, CWVWebViewConfiguration stays around longer than WebViewWebMainParts.
Since any client may retain CWVWebViewConfiguration for any reason, we cannot rely on dealloc to
clean things up. Instead, a |shutDown| method is used to tear things down in the proper order.

Bug: 733452
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I5c7b3e2f5fc81ee9d67e3a8a499301e4c814726e
Reviewed-on: https://chromium-review.googlesource.com/825444
Commit-Queue: John Wu <jzw@chromium.org>
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523991}

ios/web_view/BUILD.gn

@@ -91,6 +91,7 @@ ios_web_view_sources = [
   "internal/cwv_web_view.mm",
   "internal/cwv_web_view_configuration.mm",
   "internal/cwv_web_view_configuration_internal.h",
+  "internal/cwv_web_view_internal.h",
   "internal/ios_global_state_web_view_configuration.cc",
   "internal/language/web_view_language_model_factory.cc",
   "internal/language/web_view_language_model_factory.h",

ios/web_view/internal/autofill/cwv_autofill_controller.mm

@@ -302,6 +302,7 @@
 - (void)webStateDestroyed:(web::WebState*)webState {
   DCHECK_EQ(_webState, webState);
   [_autofillAgent detachFromWebState];
+  _autofillClient.reset();
   _webState->RemoveObserver(_webStateObserverBridge.get());
   _webStateObserverBridge.reset();
   _webState = nullptr;

ios/web_view/internal/cwv_web_view.mm

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#import "ios/web_view/public/cwv_web_view.h"
+#import "ios/web_view/internal/cwv_web_view_internal.h"
 
 #include <memory>
 #include <utility>
@@ -54,22 +54,6 @@ NSString* const kSessionStorageKey = @"sessionStorage";
 }
 
 @interface CWVWebView ()<CRWWebStateDelegate, CRWWebStateObserver> {
-  // |_configuration| must come before |_webState| here to avoid crash on
-  // deallocating CWVWebView. This is because the destructor of |_webState|
-  // indirectly accesses the BrowserState instance, which is owned by
-  // |_configuration|.
-  //
-  // Looks like the fields of the object are deallocated in the reverse order of
-  // their definition. If |_webState| comes before |_configuration|, the order
-  // of execution is like this:
-  //
-  // 1. |_configuration| is deallocated
-  // 1.1. BrowserState is deallocated
-  // 2. |_webState| is deallocated
-  // 2.1. The destructor of |_webState| indirectly accesses the BrowserState
-  //      deallocated above, causing crash.
-  //
-  // See crbug.com/712556 for the full stack trace.
   CWVWebViewConfiguration* _configuration;
   std::unique_ptr<web::WebState> _webState;
   std::unique_ptr<web::WebStateDelegateBridge> _webStateDelegate;
@@ -99,7 +83,7 @@ NSString* const kSessionStorageKey = @"sessionStorage";
 - (void)updateCurrentURLs;
 // Updates |title| property.
 - (void)updateTitle;
-// Returns a new CWVAutofillController created from |webState_|.
+// Returns a new CWVAutofillController created from |_webState|.
 - (CWVAutofillController*)newAutofillController;
 
 @end
@@ -158,6 +142,7 @@ static NSString* gUserAgentProduct = nil;
   self = [super initWithFrame:frame];
   if (self) {
     _configuration = configuration;
+    [_configuration registerWebView:self];
     _scrollView = [[CWVScrollView alloc] init];
     [self resetWebStateWithSessionStorage:nil];
   }
@@ -216,8 +201,12 @@ static NSString* gUserAgentProduct = nil;
   _javaScriptDialogPresenter->SetUIDelegate(_UIDelegate);
 }
 
-// -----------------------------------------------------------------------
-// WebStateObserver implementation.
+#pragma mark - CRWWebStateObserver
+
+- (void)webStateDestroyed:(web::WebState*)webState {
+  webState->RemoveObserver(_webStateObserver.get());
+  _webStateObserver.reset();
+}
 
 - (void)webState:(web::WebState*)webState
     navigationItemsPruned:(size_t)pruned_item_count {
@@ -515,4 +504,10 @@ static NSString* gUserAgentProduct = nil;
   self.title = base::SysUTF16ToNSString(_webState->GetTitle());
 }
 
+#pragma mark - Internal Methods
+
+- (void)shutDown {
+  _webState.reset();
+}
+
 @end

ios/web_view/internal/cwv_web_view_configuration.mm

@@ -2,14 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#import "ios/web_view/public/cwv_web_view_configuration.h"
 #import "ios/web_view/internal/cwv_web_view_configuration_internal.h"
 
+#include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/threading/thread_restrictions.h"
 #include "ios/web_view/internal/app/application_context.h"
 #import "ios/web_view/internal/cwv_preferences_internal.h"
 #import "ios/web_view/internal/cwv_user_content_controller_internal.h"
+#import "ios/web_view/internal/cwv_web_view_internal.h"
 #include "ios/web_view/internal/web_view_browser_state.h"
 #include "ios/web_view/internal/web_view_global_state_util.h"
 
@@ -20,6 +21,12 @@
 @interface CWVWebViewConfiguration () {
   // The BrowserState for this configuration.
   std::unique_ptr<ios_web_view::WebViewBrowserState> _browserState;
+
+  // Holds all CWVWebViews created with this class. Weak references.
+  NSHashTable* _webViews;
+
+  // |YES| if |shutDown| was called.
+  BOOL _wasShutDown;
 }
 
 // Initializes configuration with the specified browser state mode.
@@ -32,8 +39,15 @@
 @synthesize preferences = _preferences;
 @synthesize userContentController = _userContentController;
 
+static CWVWebViewConfiguration* defaultConfiguration;
+static CWVWebViewConfiguration* incognitoConfiguration;
+
++ (void)shutDown {
+  [defaultConfiguration shutDown];
+  [incognitoConfiguration shutDown];
+}
+
 + (instancetype)defaultConfiguration {
-  static CWVWebViewConfiguration* defaultConfiguration;
   static dispatch_once_t onceToken;
   dispatch_once(&onceToken, ^{
     auto browserState =
@@ -45,7 +59,6 @@
 }
 
 + (instancetype)incognitoConfiguration {
-  static CWVWebViewConfiguration* incognitoConfiguration;
   static dispatch_once_t onceToken;
   dispatch_once(&onceToken, ^{
     auto browserState =
@@ -75,10 +88,16 @@
 
     _userContentController =
         [[CWVUserContentController alloc] initWithConfiguration:self];
+
+    _webViews = [NSHashTable weakObjectsHashTable];
   }
   return self;
 }
 
+- (void)dealloc {
+  DCHECK(_wasShutDown);
+}
+
 #pragma mark - Public Methods
 
 - (BOOL)isPersistent {
@@ -91,4 +110,16 @@
   return _browserState.get();
 }
 
+- (void)registerWebView:(CWVWebView*)webView {
+  [_webViews addObject:webView];
+}
+
+- (void)shutDown {
+  for (CWVWebView* webView in _webViews) {
+    [webView shutDown];
+  }
+  _browserState.reset();
+  _wasShutDown = YES;
+}
+
 @end

ios/web_view/internal/cwv_web_view_configuration_internal.h

@@ -11,12 +11,26 @@ namespace ios_web_view {
 class WebViewBrowserState;
 }  // namespace ios_web_view
 
+@class CWVWebView;
+
 @interface CWVWebViewConfiguration ()
 
+// Calls |shutDown| on the singletons returned by |defaultConfiguration| and
+// |incognitoConfiguration|.
++ (void)shutDown;
+
 // The browser state associated with this configuration.
 @property(nonatomic, readonly, nonnull)
     ios_web_view::WebViewBrowserState* browserState;
 
+// Registers a |webView| so that this class can call |shutDown| on it later on.
+// Only weak references are held, so no need for de-register method.
+- (void)registerWebView:(nonnull CWVWebView*)webView;
+
+// Because Obj-C classes under ARC tend to outlive C++ classes, this method is
+// needed to cleanly tear down this object. Must be called before |dealloc|.
+- (void)shutDown;
+
 @end
 
 #endif  // IOS_WEB_VIEW_INTERNAL_CWV_WEB_VIEW_CONFIGURATION_INTERNAL_H_

ios/web_view/internal/cwv_web_view_internal.h

+// 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.
+
+#ifndef IOS_WEB_VIEW_INTERNAL_CWV_WEB_VIEW_INTERNAL_H_
+#define IOS_WEB_VIEW_INTERNAL_CWV_WEB_VIEW_INTERNAL_H_
+
+#import "ios/web_view/public/cwv_web_view.h"
+
+@interface CWVWebView ()
+
+// This is called by the associated CWVWebViewConfiguration in order to shut
+// down cleanly. See CWVWebViewConfiguration's |shutDown| method for more info.
+- (void)shutDown;
+
+@end
+
+#endif  // IOS_WEB_VIEW_INTERNAL_CWV_WEB_VIEW_INTERNAL_H_

ios/web_view/internal/web_view_web_main_parts.mm

@@ -8,6 +8,7 @@
 #include "base/path_service.h"
 #include "components/content_settings/core/common/content_settings_pattern.h"
 #include "ios/web_view/internal/app/application_context.h"
+#import "ios/web_view/internal/cwv_web_view_configuration_internal.h"
 #include "ios/web_view/internal/translate/web_view_translate_service.h"
 #include "ui/base/l10n/l10n_util_mac.h"
 #include "ui/base/resource/resource_bundle.h"
@@ -52,6 +53,7 @@ void WebViewWebMainParts::PreMainMessageLoopRun() {
 void WebViewWebMainParts::PostMainMessageLoopRun() {
   WebViewTranslateService::GetInstance()->Shutdown();
   ApplicationContext::GetInstance()->SaveState();
+  [CWVWebViewConfiguration shutDown];
 }
 
 void WebViewWebMainParts::PostDestroyThreads() {