Use WeakPtr for WebFramesManagerImpl to prevent crashes

Some crashes happen in WebFramesManagerImpl::OnFrameBecameAvailable,
probably due to a lifecycle issue of WebStateImpl/CRWWebController.
When these classes are destroyed, WebFramesManagerImpl::OnWebViewUpdate
should be called and callbacks for JS messages should be unregistered.
However this might not be true according to crash reports.

Use WeakPtr in WebFramesManagerImpl for callbacks should help suppress
crashes, and we need further investigation to fix the lifecycle issue
and revert this patch.

Bug: 990842,991616,991950
Change-Id: I746c53eaa920bd96d6820edd563d2b7f94f7eb50
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1742135Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Commit-Queue: Yi Su <mrsuyi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685214}

ios/web/js_messaging/web_frames_manager_impl.h

@@ -9,6 +9,7 @@
 
 #import <WebKit/WebKit.h>
 #include <map>
+#include "base/memory/weak_ptr.h"
 
 @class CRWWKScriptMessageRouter;
 
@@ -86,6 +87,8 @@ class WebFramesManagerImpl : public WebFramesManager {
   // Reference to the delegate.
   WebFramesManagerDelegate& delegate_;
 
+  base::WeakPtrFactory<WebFramesManagerImpl> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(WebFramesManagerImpl);
 };
 

ios/web/js_messaging/web_frames_manager_impl.mm

@@ -27,7 +27,7 @@ NSString* const kFrameBecameUnavailableMessageName = @"FrameBecameUnavailable";
 namespace web {
 
 WebFramesManagerImpl::WebFramesManagerImpl(WebFramesManagerDelegate& delegate)
-    : delegate_(delegate) {}
+    : delegate_(delegate), weak_factory_(this) {}
 
 WebFramesManagerImpl::~WebFramesManagerImpl() {
   RemoveAllWebFrames();
@@ -65,16 +65,26 @@ void WebFramesManagerImpl::OnWebViewUpdated(
   // time. This guarantees that when callbacks are invoked, |this| is always
   // valid.
   if (new_web_view) {
+    // TODO(crbug.com/991950): Clean up lifecycles of WebStateImpl and
+    // CRWWebController, ensure that callbacks will always be unregistered
+    // successfully during destruction. Remove WeakPtr here and use plain "this"
+    // instead.
+    base::WeakPtr<WebFramesManagerImpl> weak_ptr = weak_factory_.GetWeakPtr();
+
     [message_router
         setScriptMessageHandler:^(WKScriptMessage* message) {
-          this->OnFrameBecameAvailable(message);
+          if (weak_ptr) {
+            weak_ptr->OnFrameBecameAvailable(message);
+          }
         }
                            name:kFrameBecameAvailableMessageName
                         webView:new_web_view];
     [message_router
         setScriptMessageHandler:^(WKScriptMessage* message) {
           DCHECK(!delegate_.GetWebState()->IsBeingDestroyed());
-          this->OnFrameBecameUnavailable(message);
+          if (weak_ptr) {
+            weak_ptr->OnFrameBecameUnavailable(message);
+          }
         }
                            name:kFrameBecameUnavailableMessageName
                         webView:new_web_view];