[Android] simplify pop-up infobar creation

Right now popups have a strange relationship with
TabSpecificContentSettings.

Right now: the infobar is opened if:
 - A global NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED is received
 - !IsBlockageIndicated(POPUPS) in TSCS
 - There are blocked popups

At first glance this seems reasonable. However, popups do not abide
by the typical TSCS notion of blockage being indicated. On Android,
we create a new infobar for every new popup blocked. On Desktop, these
new popups do not trigger another animation of the blocked image.

So, TSCS has android specific logic which resets the "indicated" bit
every time popups are marked as blocked.

This CL simplifies this complex interaction by removing it completely.
The PopupBlockerTabHelper is now responsible for the straightforward
creation of the infobar.

This CL has no (%comment) intended behavior change.

Bug: 900645
Change-Id: I440b9b3b6f8d160f1b4d34f8fb7f566dc7dfb72d
Reviewed-on: https://chromium-review.googlesource.com/c/1308635
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605704}

chrome/browser/android/tab_android.cc

@@ -20,8 +20,6 @@
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/bookmarks/managed_bookmark_service_factory.h"
 #include "chrome/browser/browser_about_handler.h"
-#include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/offline_pages/offline_page_utils.h"
@@ -37,13 +35,12 @@
 #include "chrome/browser/sessions/tab_restore_service_factory.h"
 #include "chrome/browser/sync/glue/synced_tab_delegate_android.h"
 #include "chrome/browser/tab_contents/tab_util.h"
-#include "chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.h"
 #include "chrome/browser/ui/android/context_menu_helper.h"
 #include "chrome/browser/ui/android/infobars/infobar_container_android.h"
 #include "chrome/browser/ui/android/tab_model/tab_model.h"
 #include "chrome/browser/ui/android/tab_model/tab_model_list.h"
 #include "chrome/browser/ui/android/view_android_helper.h"
-#include "chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h"
+#include "chrome/browser/ui/browser_navigator_params.h"
 #include "chrome/browser/ui/startup/bad_flags_prompt.h"
 #include "chrome/browser/ui/tab_contents/core_tab_helper.h"
 #include "chrome/browser/ui/tab_helpers.h"
@@ -67,7 +64,6 @@
 #include "content/public/browser/interstitial_page.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
-#include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
@@ -332,36 +328,6 @@ bool TabAndroid::HasPrerenderedUrl(GURL gurl) {
   return false;
 }
 
-void TabAndroid::Observe(int type,
-                         const content::NotificationSource& source,
-                         const content::NotificationDetails& details) {
-  switch (type) {
-    case chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED: {
-      TabSpecificContentSettings* settings =
-          TabSpecificContentSettings::FromWebContents(web_contents());
-      if (!settings->IsBlockageIndicated(CONTENT_SETTINGS_TYPE_POPUPS)) {
-        // TODO(dfalcantara): Create an InfoBarDelegate to keep the
-        // PopupBlockedInfoBar logic native-side instead of straddling the JNI
-        // boundary.
-        int num_popups = 0;
-        PopupBlockerTabHelper* popup_blocker_helper =
-            PopupBlockerTabHelper::FromWebContents(web_contents());
-        if (popup_blocker_helper)
-          num_popups = popup_blocker_helper->GetBlockedPopupsCount();
-
-        if (num_popups > 0)
-          PopupBlockedInfoBarDelegate::Create(web_contents(), num_popups);
-
-        settings->SetBlockageHasBeenIndicated(CONTENT_SETTINGS_TYPE_POPUPS);
-      }
-      break;
-    }
-    default:
-      NOTREACHED() << "Unexpected notification " << type;
-      break;
-  }
-}
-
 void TabAndroid::OnFaviconUpdated(favicon::FaviconDriver* favicon_driver,
                                   NotificationIconType notification_icon_type,
                                   const GURL& icon_url,
@@ -418,11 +384,6 @@ void TabAndroid::InitWebContents(
   web_contents_delegate_->LoadProgressChanged(web_contents(), 0);
   web_contents()->SetDelegate(web_contents_delegate_.get());
 
-  notification_registrar_.Add(
-      this,
-      chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
-      content::Source<content::WebContents>(web_contents()));
-
   favicon::FaviconDriver* favicon_driver =
       favicon::ContentFaviconDriver::FromWebContents(web_contents_.get());
 
@@ -466,10 +427,6 @@ void TabAndroid::DestroyWebContents(JNIEnv* env,
   if (web_contents()->GetNativeView())
     web_contents()->GetNativeView()->GetLayer()->RemoveFromParent();
 
-  notification_registrar_.Remove(
-      this,
-      chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
-      content::Source<content::WebContents>(web_contents()));
   WebContentsObserver::Observe(nullptr);
 
   favicon::FaviconDriver* favicon_driver =

chrome/browser/android/tab_android.h

@@ -23,8 +23,6 @@
 #include "components/infobars/core/infobar_manager.h"
 #include "components/omnibox/browser/location_bar_model.h"
 #include "components/sessions/core/session_id.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
 #include "third_party/blink/public/platform/media_download_in_product_help.mojom.h"
@@ -54,7 +52,6 @@ class PrerenderManager;
 }
 
 class TabAndroid : public CoreTabHelperDelegate,
-                   public content::NotificationObserver,
                    public favicon::FaviconDriverObserver,
                    public content::WebContentsObserver {
  public:
@@ -123,11 +120,6 @@ class TabAndroid : public CoreTabHelperDelegate,
 
   bool HasPrerenderedUrl(GURL gurl);
 
-  // Overridden from NotificationObserver:
-  void Observe(int type,
-               const content::NotificationSource& source,
-               const content::NotificationDetails& details) override;
-
   // Overridden from favicon::FaviconDriverObserver:
   void OnFaviconUpdated(favicon::FaviconDriver* favicon_driver,
                         NotificationIconType notification_icon_type,
@@ -313,8 +305,6 @@ class TabAndroid : public CoreTabHelperDelegate,
   // Identifier of the window the tab is in.
   SessionID session_window_id_;
 
-  content::NotificationRegistrar notification_registrar_;
-
   scoped_refptr<cc::Layer> content_layer_;
   android::TabContentManager* tab_content_manager_;
 

chrome/browser/content_settings/tab_specific_content_settings.cc

@@ -336,17 +336,6 @@ void TabSpecificContentSettings::OnContentBlockedWithDetail(
 
   ContentSettingsStatus& status = content_settings_status_[type];
 
-#if defined(OS_ANDROID)
-  if (type == CONTENT_SETTINGS_TYPE_POPUPS) {
-    // For Android we do not have a persistent button that will always be
-    // visible for blocked popups.  Instead we have info bars which could be
-    // dismissed.  Have to clear the blocked state so we properly notify the
-    // relevant pieces again.
-    status.blocked = false;
-    status.blockage_indicated_to_user = false;
-  }
-#endif
-
   if (!status.blocked) {
     status.blocked = true;
     // TODO: it would be nice to have a way of mocking this in tests.

chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc

@@ -8,7 +8,9 @@
 #include <string>
 
 #include "base/metrics/histogram_macros.h"
+#include "build/build_config.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
+#include "chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.h"
 #include "chrome/browser/ui/blocked_content/blocked_window_params.h"
 #include "chrome/browser/ui/blocked_content/list_item_position.h"
 #include "chrome/browser/ui/blocked_content/popup_tracker.h"
@@ -102,6 +104,12 @@ void PopupBlockerTabHelper::AddBlockedPopup(
       OnContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS);
   for (auto& observer : observers_)
     observer.BlockedPopupAdded(id, blocked_popups_[id]->params.url);
+
+#if defined(OS_ANDROID)
+  // Should replace existing popup infobars, with an updated count of how many
+  // popups have been blocked.
+  PopupBlockedInfoBarDelegate::Create(web_contents(), GetBlockedPopupsCount());
+#endif
 }
 
 void PopupBlockerTabHelper::ShowBlockedPopup(

chrome/browser/ui/blocked_content/safe_browsing_triggered_popup_blocker_unittest.cc

@@ -15,6 +15,7 @@
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
+#include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/safe_browsing/test_safe_browsing_service.h"
 #include "chrome/browser/subresource_filter/chrome_subresource_filter_client.h"
 #include "chrome/browser/ui/blocked_content/popup_blocker.h"
@@ -78,6 +79,7 @@ class SafeBrowsingTriggeredPopupBlockerTest
 
     scoped_feature_list_ = DefaultFeatureList();
     PopupBlockerTabHelper::CreateForWebContents(web_contents());
+    InfoBarService::CreateForWebContents(web_contents());
     TabSpecificContentSettings::CreateForWebContents(web_contents());
     popup_blocker_ =
         SafeBrowsingTriggeredPopupBlocker::FromWebContents(web_contents());