Fix framebust omnibox toast

Currently, the icon persists across navigations, which is wrong.

Bug: 894955
Change-Id: I5e2028668a249c0baeb3976297c499df3e61b3b3
Reviewed-on: https://chromium-review.googlesource.com/c/1279208
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599982}

chrome/browser/ui/blocked_content/framebust_block_tab_helper.cc

@@ -5,6 +5,9 @@
 #include "chrome/browser/ui/blocked_content/framebust_block_tab_helper.h"
 
 #include "base/logging.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/notification_service.h"
 
 FramebustBlockTabHelper::~FramebustBlockTabHelper() = default;
 
@@ -29,11 +32,9 @@ void FramebustBlockTabHelper::OnBlockedUrlClicked(size_t index) {
   const GURL& url = blocked_urls_[index];
   if (!callbacks_[index].is_null())
     std::move(callbacks_[index]).Run(url, index, total_size);
-  web_contents_->OpenURL(content::OpenURLParams(
+  web_contents()->OpenURL(content::OpenURLParams(
       url, content::Referrer(), WindowOpenDisposition::CURRENT_TAB,
       ui::PAGE_TRANSITION_LINK, false));
-  blocked_urls_.clear();
-  callbacks_.clear();
 }
 
 void FramebustBlockTabHelper::AddObserver(Observer* observer) {
@@ -46,4 +47,26 @@ void FramebustBlockTabHelper::RemoveObserver(const Observer* observer) {
 
 FramebustBlockTabHelper::FramebustBlockTabHelper(
     content::WebContents* web_contents)
-    : web_contents_(web_contents) {}
+    : content::WebContentsObserver(web_contents) {}
+
+void FramebustBlockTabHelper::DidFinishNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (!navigation_handle->IsInMainFrame() ||
+      !navigation_handle->HasCommitted() ||
+      navigation_handle->IsSameDocument()) {
+    return;
+  }
+  blocked_urls_.clear();
+  callbacks_.clear();
+  animation_has_run_ = false;
+
+  // TODO(csharrison): It is a bit ugly that this tab helper has to notify this
+  // change directly. Consider improving this by integrating framebust
+  // information with the TabSpecificContentSetting class. This may be
+  // challenging, since popups and framebusts are controlled by the same content
+  // setting.
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
+      content::Source<content::WebContents>(web_contents()),
+      content::NotificationService::NoDetails());
+}

chrome/browser/ui/blocked_content/framebust_block_tab_helper.h

@@ -10,13 +10,19 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 #include "url/gurl.h"
 
+namespace content {
+class NavigationHandle;
+}
+
 // A tab helper that keeps track of blocked Framebusts that happened on each
 // page. Only used for the desktop version of the blocked Framebust UI.
 class FramebustBlockTabHelper
-    : public content::WebContentsUserData<FramebustBlockTabHelper> {
+    : public content::WebContentsObserver,
+      public content::WebContentsUserData<FramebustBlockTabHelper> {
  public:
   using ClickCallback = base::OnceCallback<
       void(const GURL&, size_t /* index */, size_t /* total_size */)>;
@@ -61,7 +67,9 @@ class FramebustBlockTabHelper
 
   explicit FramebustBlockTabHelper(content::WebContents* web_contents);
 
-  content::WebContents* web_contents_;
+  // content::WebContentsObserver:
+  void DidFinishNavigation(
+      content::NavigationHandle* navigation_handle) override;
 
   // Remembers all the currently blocked URLs. This is cleared on each
   // navigation.

chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc

@@ -515,13 +515,7 @@ TEST_F(BlockTabUnderTest, TabUnderWithSubsequentGesture_IsNotBlocked) {
   // A subsequent navigation should be allowed, even if it is classified as a
   // suspicious redirect.
   EXPECT_TRUE(NavigateAndCommitWithoutGesture(GURL("https://example.test2/")));
-#if defined(OS_ANDROID)
   ExpectUIShown(false);
-#else
-  EXPECT_EQ(1u, FramebustBlockTabHelper::FromWebContents(web_contents())
-                    ->blocked_urls()
-                    .size());
-#endif
 }
 
 TEST_F(BlockTabUnderTest, MultipleRedirectAttempts_AreBlocked) {

chrome/browser/ui/content_settings/framebust_block_browsertest.cc

@@ -314,3 +314,36 @@ IN_PROC_BROWSER_TEST_F(FramebustBlockBrowserTest,
   observer.Wait();
   EXPECT_TRUE(GetFramebustTabHelper()->blocked_urls().empty());
 }
+
+// Regression test for https://crbug.com/894955, where the framebust UI would
+// persist on subsequent navigations.
+IN_PROC_BROWSER_TEST_F(FramebustBlockBrowserTest,
+                       FramebustBlocked_SubsequentNavigation_NoUI) {
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/iframe.html"));
+
+  GURL child_url = embedded_test_server()->GetURL("a.com", "/title1.html");
+  NavigateIframeToUrlWithoutGesture(GetWebContents(), "test", child_url);
+
+  content::RenderFrameHost* child =
+      content::ChildFrameAt(GetWebContents()->GetMainFrame(), 0);
+  EXPECT_EQ(child_url, child->GetLastCommittedURL());
+
+  GURL redirect_url = embedded_test_server()->GetURL("b.com", "/title1.html");
+
+  base::RunLoop block_waiter;
+  blocked_url_added_closure_ = block_waiter.QuitClosure();
+  child->ExecuteJavaScriptForTests(base::ASCIIToUTF16(base::StringPrintf(
+      "window.top.location = '%s';", redirect_url.spec().c_str())));
+  block_waiter.Run();
+  EXPECT_TRUE(base::ContainsValue(GetFramebustTabHelper()->blocked_urls(),
+                                  redirect_url));
+
+  // Now, navigate away and check that the UI went away.
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title2.html"));
+
+  // TODO(csharrison): Ideally we could query the actual UI here. For now, just
+  // look at the internal state of the framebust tab helper.
+  EXPECT_FALSE(GetFramebustTabHelper()->HasBlockedUrls());
+}