Fix crash in handling AssistantWebViewImpl suppressed navigation.

It is potentially unsafe to destroy AssistantWebViewImpl during
handling of suppressed navigation events. This was not previously an
issue for Assistant as the Content Service backed implementation
notified handlers of this event over mojo, so destruction was performed
after the suppression sequence had completed.

Note that this is currently only affecting proactive suggestions as only
proactive suggestions currently uses target="_blank" for its links in
conjunction w/ the destruction sequence which triggers code path [1] as
opposed to code path [2].

[1] http://shortn/_H18kMl1jSt
[2] http://shortn/_J5ss5Vjirn

Bug: b:148484855
Change-Id: I9e42c371a3f53e16c6112bd4f7506ea2de749f2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2026689Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/master@{#737062}

chrome/browser/ui/ash/assistant/assistant_web_view_impl.cc

@@ -72,11 +72,9 @@ bool AssistantWebViewImpl::IsWebContentsCreationOverridden(
     const std::string& frame_name,
     const GURL& target_url) {
   if (params_.suppress_navigation) {
-    for (auto& observer : observers_) {
-      observer.DidSuppressNavigation(target_url,
-                                     WindowOpenDisposition::NEW_FOREGROUND_TAB,
-                                     /*from_user_gesture=*/true);
-    }
+    NotifyDidSuppressNavigation(target_url,
+                                WindowOpenDisposition::NEW_FOREGROUND_TAB,
+                                /*from_user_gesture=*/true);
     return true;
   }
   return content::WebContentsDelegate::IsWebContentsCreationOverridden(
@@ -88,10 +86,8 @@ content::WebContents* AssistantWebViewImpl::OpenURLFromTab(
     content::WebContents* source,
     const content::OpenURLParams& params) {
   if (params_.suppress_navigation) {
-    for (auto& observer : observers_) {
-      observer.DidSuppressNavigation(params.url, params.disposition,
-                                     /*from_user_gesture=*/true);
-    }
+    NotifyDidSuppressNavigation(params.url, params.disposition,
+                                params.user_gesture);
     return nullptr;
   }
   return content::WebContentsDelegate::OpenURLFromTab(source, params);
@@ -192,6 +188,33 @@ void AssistantWebViewImpl::InitLayout(Profile* profile) {
   AddChildView(web_view_.get());
 }
 
+void AssistantWebViewImpl::NotifyDidSuppressNavigation(
+    const GURL& url,
+    WindowOpenDisposition disposition,
+    bool from_user_gesture) {
+  // Note that we post notification to |observers_| as an observer may cause
+  // |this| to be deleted during handling of the event which is unsafe to do
+  // until the original navigation sequence has been completed.
+  base::SequencedTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          [](const base::WeakPtr<AssistantWebViewImpl>& self, GURL url,
+             WindowOpenDisposition disposition, bool from_user_gesture) {
+            if (self) {
+              for (auto& observer : self->observers_) {
+                observer.DidSuppressNavigation(url, disposition,
+                                               from_user_gesture);
+
+                // We need to check |self| to confirm that |observer| did not
+                // delete |this|. If |this| is deleted, we quit.
+                if (!self)
+                  return;
+              }
+            }
+          },
+          weak_factory_.GetWeakPtr(), url, disposition, from_user_gesture));
+}
+
 void AssistantWebViewImpl::UpdateCanGoBack() {
   const bool can_go_back = web_contents_->GetController().CanGoBack();
   if (can_go_back_ == can_go_back)

chrome/browser/ui/ash/assistant/assistant_web_view_impl.h

@@ -70,6 +70,10 @@ class AssistantWebViewImpl : public ash::AssistantWebView2,
   void InitWebContents(Profile* profile);
   void InitLayout(Profile* profile);
 
+  void NotifyDidSuppressNavigation(const GURL& url,
+                                   WindowOpenDisposition disposition,
+                                   bool from_user_gesture);
+
   void UpdateCanGoBack();
 
   const InitParams params_;
@@ -81,6 +85,8 @@ class AssistantWebViewImpl : public ash::AssistantWebView2,
   bool can_go_back_ = false;
 
   base::ObserverList<Observer> observers_;
+
+  base::WeakPtrFactory<AssistantWebViewImpl> weak_factory_{this};
 };
 
 #endif  // CHROME_BROWSER_UI_ASH_ASSISTANT_ASSISTANT_WEB_VIEW_IMPL_H_