Remove NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED
In production, this had only one listener (LocationBar) but many
callsites. Update callsites to more directly access LocationBar.
In tests, replace by listening for visibility changes in the relevant
location bar icon.
Bug: 268984
Change-Id: I552c9ea27d606e992c099b63d14ca123820658b6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1682795
Reviewed-by: Scott Violet <[email protected]>
Commit-Queue: Evan Stade <[email protected]>
Cr-Commit-Position: refs/heads/master@{#675288}
diff --git a/chrome/browser/chrome_notification_types.h b/chrome/browser/chrome_notification_types.h
index 33fe370..9176955 100644
--- a/chrome/browser/chrome_notification_types.h
+++ b/chrome/browser/chrome_notification_types.h
@@ -191,12 +191,6 @@
// Source is the WebContents that holds the print job.
NOTIFICATION_PRINT_JOB_RELEASED,
- // Content Settings --------------------------------------------------------
-
- // Sent when content settings change for a tab. The source is a
- // content::WebContents object, the details are None.
- NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
-
// Cookies -----------------------------------------------------------------
#if BUILDFLAG(ENABLE_EXTENSIONS)
diff --git a/chrome/browser/content_settings/chrome_content_settings_utils.cc b/chrome/browser/content_settings/chrome_content_settings_utils.cc
index 663e7c1..12037091 100644
--- a/chrome/browser/content_settings/chrome_content_settings_utils.cc
+++ b/chrome/browser/content_settings/chrome_content_settings_utils.cc
@@ -6,6 +6,15 @@
#include "base/metrics/histogram_macros.h"
+#if !defined(OS_ANDROID)
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/location_bar/location_bar.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "content/public/browser/web_contents.h"
+#endif
+
namespace content_settings {
void RecordMixedScriptAction(MixedScriptAction action) {
@@ -23,4 +32,19 @@
POPUPS_ACTION_COUNT);
}
+void UpdateLocationBarUiForWebContents(content::WebContents* web_contents) {
+#if !defined(OS_ANDROID)
+ Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
+ if (!browser)
+ return;
+
+ if (browser->tab_strip_model()->GetActiveWebContents() != web_contents)
+ return;
+
+ LocationBar* location_bar = browser->window()->GetLocationBar();
+ if (location_bar)
+ location_bar->UpdateContentSettingsIcons();
+#endif
+}
+
} // namespace content_settings
diff --git a/chrome/browser/content_settings/chrome_content_settings_utils.h b/chrome/browser/content_settings/chrome_content_settings_utils.h
index 50429a6..de33fa2 100644
--- a/chrome/browser/content_settings/chrome_content_settings_utils.h
+++ b/chrome/browser/content_settings/chrome_content_settings_utils.h
@@ -5,10 +5,16 @@
#ifndef CHROME_BROWSER_CONTENT_SETTINGS_CHROME_CONTENT_SETTINGS_UTILS_H_
#define CHROME_BROWSER_CONTENT_SETTINGS_CHROME_CONTENT_SETTINGS_UTILS_H_
+#include "build/build_config.h"
+
// Put utility functions only used by //chrome code here. If a function declared
// here would be meaningfully shared with other platforms, consider moving it to
// components/content_settings/core/browser/content_settings_utils.h.
+namespace content {
+class WebContents;
+} // namespace content
+
namespace content_settings {
// UMA histogram for the mixed script shield. The enum values correspond to
@@ -54,6 +60,9 @@
void RecordPopupsAction(PopupsAction action);
+// Calls UpdateContentSettingsIcons on the |LocationBar| for |web_contents|.
+void UpdateLocationBarUiForWebContents(content::WebContents* web_contents);
+
} // namespace content_settings
#endif // CHROME_BROWSER_CONTENT_SETTINGS_CHROME_CONTENT_SETTINGS_UTILS_H_
diff --git a/chrome/browser/content_settings/content_settings_browsertest.cc b/chrome/browser/content_settings/content_settings_browsertest.cc
index bc32d78..35f8704 100644
--- a/chrome/browser/content_settings/content_settings_browsertest.cc
+++ b/chrome/browser/content_settings/content_settings_browsertest.cc
@@ -12,7 +12,6 @@
#include "base/test/scoped_feature_list.h"
#include "build/build_config.h"
#include "chrome/browser/browsing_data/browsing_data_cookie_helper.h"
-#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/content_settings/cookie_settings_factory.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/content_settings/tab_specific_content_settings.h"
@@ -21,10 +20,12 @@
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/view_ids.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/interactive_test_utils.h"
#include "chrome/test/base/test_launcher_utils.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/content_settings/core/browser/cookie_settings.h"
@@ -33,8 +34,6 @@
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/navigation_handle.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_service.h"
#include "content/public/browser/plugin_service.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h"
@@ -746,12 +745,6 @@
TabSpecificContentSettings* tab_settings =
TabSpecificContentSettings::FromWebContents(web_contents);
- content::WindowedNotificationObserver javascript_content_blocked_observer(
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- base::BindRepeating(&TabSpecificContentSettings::IsContentBlocked,
- base::Unretained(tab_settings),
- CONTENT_SETTINGS_TYPE_JAVASCRIPT));
-
base::string16 expected_title(base::ASCIIToUTF16("Failed"));
content::TitleWatcher title_watcher(web_contents, expected_title);
title_watcher.AlsoWaitForTitle(base::ASCIIToUTF16("Imported"));
@@ -759,7 +752,8 @@
ui_test_utils::NavigateToURL(browser(), http_url);
// The import must be blocked.
- javascript_content_blocked_observer.Wait();
+ ui_test_utils::WaitForViewVisibility(
+ browser(), VIEW_ID_CONTENT_SETTING_JAVASCRIPT, true);
EXPECT_TRUE(tab_settings->IsContentBlocked(CONTENT_SETTINGS_TYPE_JAVASCRIPT));
EXPECT_EQ(expected_title, title_watcher.WaitAndGetTitle());
}
@@ -925,24 +919,16 @@
// before the blocked content can be reported to the browser process.
// See http://crbug.com/306702.
// Therefore, when expecting blocked content, we must wait until it has been
- // reported by checking IsContentBlocked() when notified that
- // NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED. (It is not sufficient to wait
- // for just the notification because the same notification is reported for
- // other reasons and the notification contains no indication of what
- // caused it.)
- content::WindowedNotificationObserver javascript_content_blocked_observer(
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- base::Bind(&TabSpecificContentSettings::IsContentBlocked,
- base::Unretained(tab_settings),
- CONTENT_SETTINGS_TYPE_JAVASCRIPT));
-
+ // reported by waiting for the appropriate icon to appear in the location
+ // bar, and checking IsContentBlocked().
ui_test_utils::NavigateToURL(browser(), https_server_.GetURL(path));
// Always wait for the page to load.
EXPECT_EQ(expected_title, title_watcher.WaitAndGetTitle());
if (expect_is_javascript_content_blocked) {
- javascript_content_blocked_observer.Wait();
+ ui_test_utils::WaitForViewVisibility(
+ browser(), VIEW_ID_CONTENT_SETTING_JAVASCRIPT, true);
} else {
// Since there is no notification that content is not blocked and no
// content is blocked when |expect_is_javascript_content_blocked| is
diff --git a/chrome/browser/content_settings/tab_specific_content_settings.cc b/chrome/browser/content_settings/tab_specific_content_settings.cc
index f521d5f78..604bf66d 100644
--- a/chrome/browser/content_settings/tab_specific_content_settings.cc
+++ b/chrome/browser/content_settings/tab_specific_content_settings.cc
@@ -20,7 +20,6 @@
#include "chrome/browser/browsing_data/browsing_data_indexed_db_helper.h"
#include "chrome/browser/browsing_data/browsing_data_local_storage_helper.h"
#include "chrome/browser/browsing_data/cookies_tree_model.h"
-#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/content_settings/chrome_content_settings_utils.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h"
@@ -42,8 +41,6 @@
#include "content/public/browser/navigation_details.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/navigation_handle.h"
-#include "content/public/browser/notification_registrar.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"
@@ -335,11 +332,7 @@
if (!status.blocked) {
status.blocked = true;
- // TODO: it would be nice to have a way of mocking this in tests.
- content::NotificationService::current()->Notify(
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- content::Source<WebContents>(web_contents()),
- content::NotificationService::NoDetails());
+ content_settings::UpdateLocationBarUiForWebContents(web_contents());
if (type == CONTENT_SETTINGS_TYPE_MIXEDSCRIPT) {
content_settings::RecordMixedScriptAction(
@@ -394,12 +387,8 @@
access_changed = true;
}
- if (access_changed) {
- content::NotificationService::current()->Notify(
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- content::Source<WebContents>(web_contents()),
- content::NotificationService::NoDetails());
- }
+ if (access_changed)
+ content_settings::UpdateLocationBarUiForWebContents(web_contents());
}
void TabSpecificContentSettings::OnCookiesRead(
@@ -564,10 +553,7 @@
const GURL& requesting_origin,
bool allowed) {
geolocation_usages_state_.OnPermissionSet(requesting_origin, allowed);
- content::NotificationService::current()->Notify(
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- content::Source<WebContents>(web_contents()),
- content::NotificationService::NoDetails());
+ content_settings::UpdateLocationBarUiForWebContents(web_contents());
}
#if defined(OS_ANDROID) || defined(OS_CHROMEOS)
@@ -665,10 +651,7 @@
if (microphone_camera_state_ != new_microphone_camera_state) {
microphone_camera_state_ = new_microphone_camera_state;
- content::NotificationService::current()->Notify(
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- content::Source<WebContents>(web_contents()),
- content::NotificationService::NoDetails());
+ content_settings::UpdateLocationBarUiForWebContents(web_contents());
}
}
@@ -695,10 +678,7 @@
}
microphone_camera_state_ = MICROPHONE_CAMERA_NOT_ACCESSED;
load_plugins_link_enabled_ = true;
- content::NotificationService::current()->Notify(
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- content::Source<WebContents>(web_contents()),
- content::NotificationService::NoDetails());
+ content_settings::UpdateLocationBarUiForWebContents(web_contents());
}
void TabSpecificContentSettings::ClearNavigationRelatedContentSettings() {
@@ -711,10 +691,7 @@
status.blocked = false;
status.allowed = false;
}
- content::NotificationService::current()->Notify(
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- content::Source<WebContents>(web_contents()),
- content::NotificationService::NoDetails());
+ content_settings::UpdateLocationBarUiForWebContents(web_contents());
}
void TabSpecificContentSettings::FlashDownloadBlocked() {
@@ -726,10 +703,7 @@
ContentSettingsStatus& status =
content_settings_status_[CONTENT_SETTINGS_TYPE_POPUPS];
status.blocked = false;
- content::NotificationService::current()->Notify(
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- content::Source<WebContents>(web_contents()),
- content::NotificationService::NoDetails());
+ content_settings::UpdateLocationBarUiForWebContents(web_contents());
}
void TabSpecificContentSettings::OnAudioBlocked() {
diff --git a/chrome/browser/download/download_request_limiter.cc b/chrome/browser/download/download_request_limiter.cc
index 1cf321f..9b375961 100644
--- a/chrome/browser/download/download_request_limiter.cc
+++ b/chrome/browser/download/download_request_limiter.cc
@@ -11,7 +11,7 @@
#include "base/stl_util.h"
#include "base/task/post_task.h"
#include "build/build_config.h"
-#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/content_settings/chrome_content_settings_utils.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/download/download_permission_request.h"
#include "chrome/browser/permissions/permission_request_manager.h"
@@ -25,7 +25,6 @@
#include "content/public/browser/navigation_controller.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_process_host.h"
#include "content/public/browser/resource_dispatcher_host.h"
#include "content/public/browser/web_contents.h"
@@ -433,10 +432,7 @@
return;
}
- content::NotificationService::current()->Notify(
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- content::Source<content::WebContents>(web_contents()),
- content::NotificationService::NoDetails());
+ content_settings::UpdateLocationBarUiForWebContents(web_contents());
}
bool DownloadRequestLimiter::TabDownloadState::IsNavigationRestricted(
diff --git a/chrome/browser/extensions/app_process_apitest.cc b/chrome/browser/extensions/app_process_apitest.cc
index fd1b73d..242b95c 100644
--- a/chrome/browser/extensions/app_process_apitest.cc
+++ b/chrome/browser/extensions/app_process_apitest.cc
@@ -16,7 +16,9 @@
#include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/view_ids.h"
#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/interactive_test_utils.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/sync/model/string_ordinal.h"
#include "content/public/browser/navigation_entry.h"
@@ -643,17 +645,12 @@
ui_test_utils::NavigateToURL(
browser(), GetTestBaseURL("app_process").Resolve("path3/container.html"));
+ ui_test_utils::WaitForViewVisibility(browser(), VIEW_ID_CONTENT_SETTING_POPUP,
+ true);
WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
PopupBlockerTabHelper* popup_blocker_tab_helper =
PopupBlockerTabHelper::FromWebContents(tab);
- if (!popup_blocker_tab_helper->GetBlockedPopupsCount()) {
- content::WindowedNotificationObserver observer(
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- content::NotificationService::AllSources());
- observer.Wait();
- }
-
EXPECT_EQ(1u, popup_blocker_tab_helper->GetBlockedPopupsCount());
}
diff --git a/chrome/browser/media/webrtc/media_stream_capture_indicator.cc b/chrome/browser/media/webrtc/media_stream_capture_indicator.cc
index ec0bf473..25f9bb09 100644
--- a/chrome/browser/media/webrtc/media_stream_capture_indicator.cc
+++ b/chrome/browser/media/webrtc/media_stream_capture_indicator.cc
@@ -15,14 +15,13 @@
#include "build/build_config.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/browser_process.h"
-#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/content_settings/chrome_content_settings_utils.h"
#include "chrome/browser/status_icons/status_icon.h"
#include "chrome/browser/status_icons/status_tray.h"
#include "chrome/browser/tab_contents/tab_util.h"
#include "components/url_formatter/elide_url.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/content_browser_client.h"
-#include "content/public/browser/notification_service.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_delegate.h"
#include "content/public/browser/web_contents_observer.h"
@@ -216,10 +215,7 @@
if (web_contents()) {
web_contents()->NotifyNavigationStateChanged(content::INVALIDATE_TYPE_TAB);
- content::NotificationService::current()->Notify(
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- content::Source<WebContents>(web_contents()),
- content::NotificationService::NoDetails());
+ content_settings::UpdateLocationBarUiForWebContents(web_contents());
}
indicator_->UpdateNotificationUserInterface();
diff --git a/chrome/browser/ui/blocked_content/framebust_block_tab_helper.cc b/chrome/browser/ui/blocked_content/framebust_block_tab_helper.cc
index f9ced124..7782b403 100644
--- a/chrome/browser/ui/blocked_content/framebust_block_tab_helper.cc
+++ b/chrome/browser/ui/blocked_content/framebust_block_tab_helper.cc
@@ -5,20 +5,8 @@
#include "chrome/browser/ui/blocked_content/framebust_block_tab_helper.h"
#include "base/logging.h"
-#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/content_settings/chrome_content_settings_utils.h"
#include "content/public/browser/navigation_handle.h"
-#include "content/public/browser/notification_service.h"
-
-namespace {
-
-void UpdateLocationBarUI(content::WebContents* contents) {
- content::NotificationService::current()->Notify(
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- content::Source<content::WebContents>(contents),
- content::NotificationService::NoDetails());
-}
-
-} // namespace
FramebustBlockTabHelper::~FramebustBlockTabHelper() = default;
@@ -29,7 +17,7 @@
DCHECK_EQ(blocked_urls_.size(), callbacks_.size());
manager_.NotifyObservers(0 /* id */, blocked_url);
- UpdateLocationBarUI(web_contents());
+ content_settings::UpdateLocationBarUiForWebContents(web_contents());
}
bool FramebustBlockTabHelper::HasBlockedUrls() const {
@@ -61,7 +49,7 @@
blocked_urls_.clear();
callbacks_.clear();
- UpdateLocationBarUI(web_contents());
+ content_settings::UpdateLocationBarUiForWebContents(web_contents());
}
WEB_CONTENTS_USER_DATA_KEY_IMPL(FramebustBlockTabHelper)
diff --git a/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc b/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc
index 7df5f3b..7f43575 100644
--- a/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc
+++ b/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc
@@ -30,6 +30,7 @@
#include "chrome/browser/ui/login/login_handler.h"
#include "chrome/browser/ui/login/login_handler_test_utils.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/view_ids.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h"
@@ -221,12 +222,8 @@
// Launch the blocked popup.
PopupBlockerTabHelper* popup_blocker_helper =
PopupBlockerTabHelper::FromWebContents(web_contents);
- if (!popup_blocker_helper->GetBlockedPopupsCount()) {
- content::WindowedNotificationObserver observer(
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- content::NotificationService::AllSources());
- observer.Wait();
- }
+ ui_test_utils::WaitForViewVisibility(browser, VIEW_ID_CONTENT_SETTING_POPUP,
+ true);
EXPECT_EQ(1u, popup_blocker_helper->GetBlockedPopupsCount());
std::map<int32_t, GURL> blocked_requests =
popup_blocker_helper->GetBlockedPopupRequests();
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index 95dbd565..b08b03ac 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -448,8 +448,6 @@
registrar_.Add(this, chrome::NOTIFICATION_BROWSER_THEME_CHANGED,
content::Source<ThemeService>(
ThemeServiceFactory::GetForProfile(profile_)));
- registrar_.Add(this, chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- content::NotificationService::AllSources());
profile_pref_registrar_.Init(profile_->GetPrefs());
profile_pref_registrar_.Add(
@@ -2001,24 +1999,8 @@
void Browser::Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
- switch (type) {
- case chrome::NOTIFICATION_BROWSER_THEME_CHANGED:
- window()->UserChangedTheme(BrowserThemeChangeType::kBrowserTheme);
- break;
-
- case chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED: {
- WebContents* web_contents = content::Source<WebContents>(source).ptr();
- if (web_contents == tab_strip_model_->GetActiveWebContents()) {
- LocationBar* location_bar = window()->GetLocationBar();
- if (location_bar)
- location_bar->UpdateContentSettingsIcons();
- }
- break;
- }
-
- default:
- NOTREACHED() << "Got a notification we didn't register for.";
- }
+ DCHECK_EQ(chrome::NOTIFICATION_BROWSER_THEME_CHANGED, type);
+ window()->UserChangedTheme(BrowserThemeChangeType::kBrowserTheme);
}
///////////////////////////////////////////////////////////////////////////////
diff --git a/chrome/browser/ui/content_settings/content_setting_bubble_model.cc b/chrome/browser/ui/content_settings/content_setting_bubble_model.cc
index 75690a38..794d8a9 100644
--- a/chrome/browser/ui/content_settings/content_setting_bubble_model.cc
+++ b/chrome/browser/ui/content_settings/content_setting_bubble_model.cc
@@ -16,7 +16,6 @@
#include "base/stl_util.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/browser_process.h"
-#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/content_settings/chrome_content_settings_utils.h"
#include "chrome/browser/content_settings/cookie_settings_factory.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
@@ -54,7 +53,6 @@
#include "components/subresource_filter/core/browser/subresource_filter_constants.h"
#include "components/subresource_filter/core/browser/subresource_filter_features.h"
#include "components/url_formatter/elide_url.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"
@@ -423,10 +421,7 @@
// then we remove it.
auto* settings = TabSpecificContentSettings::FromWebContents(web_contents());
settings->ClearPendingProtocolHandler();
- content::NotificationService::current()->Notify(
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- content::Source<WebContents>(web_contents()),
- content::NotificationService::NoDetails());
+ content_settings::UpdateLocationBarUiForWebContents(web_contents());
}
void ContentSettingRPHBubbleModel::RegisterProtocolHandler() {
diff --git a/chrome/browser/ui/content_settings/content_setting_image_model_unittest.cc b/chrome/browser/ui/content_settings/content_setting_image_model_unittest.cc
index 072b0422..b297539 100644
--- a/chrome/browser/ui/content_settings/content_setting_image_model_unittest.cc
+++ b/chrome/browser/ui/content_settings/content_setting_image_model_unittest.cc
@@ -33,39 +33,7 @@
return !model.GetIcon(gfx::kPlaceholderColor).IsEmpty();
}
-// Forward all NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED to the specified
-// ContentSettingImageModel.
-class NotificationForwarder : public content::NotificationObserver {
- public:
- explicit NotificationForwarder(ContentSettingImageModel* model)
- : model_(model) {
- registrar_.Add(this,
- chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
- content::NotificationService::AllSources());
- }
- ~NotificationForwarder() override {}
-
- void clear() {
- registrar_.RemoveAll();
- }
-
- void Observe(int type,
- const content::NotificationSource& source,
- const content::NotificationDetails& details) override {
- if (type == chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED) {
- model_->Update(content::Source<content::WebContents>(source).ptr());
- }
- }
-
- private:
- content::NotificationRegistrar registrar_;
- ContentSettingImageModel* model_;
-
- DISALLOW_COPY_AND_ASSIGN(NotificationForwarder);
-};
-
-class ContentSettingImageModelTest : public ChromeRenderViewHostTestHarness {
-};
+using ContentSettingImageModelTest = ChromeRenderViewHostTestHarness;
TEST_F(ContentSettingImageModelTest, Update) {
TabSpecificContentSettings::CreateForWebContents(web_contents());
@@ -325,13 +293,12 @@
// Regression test for http://crbug.com/161854.
TEST_F(ContentSettingImageModelTest, NULLTabSpecificContentSettings) {
- auto content_setting_image_model =
- ContentSettingImageModel::CreateForContentType(
- ContentSettingImageModel::ImageType::IMAGES);
- NotificationForwarder forwarder(content_setting_image_model.get());
+ EXPECT_EQ(nullptr,
+ TabSpecificContentSettings::FromWebContents(web_contents()));
// Should not crash.
- TabSpecificContentSettings::CreateForWebContents(web_contents());
- forwarder.clear();
+ ContentSettingImageModel::CreateForContentType(
+ ContentSettingImageModel::ImageType::IMAGES)
+ ->Update(web_contents());
}
TEST_F(ContentSettingImageModelTest, SubresourceFilter) {
diff --git a/chrome/browser/ui/view_ids.h b/chrome/browser/ui/view_ids.h
index 234cb55..8af18a9f 100644
--- a/chrome/browser/ui/view_ids.h
+++ b/chrome/browser/ui/view_ids.h
@@ -62,6 +62,10 @@
VIEW_ID_MIGRATE_LOCAL_CREDIT_CARD_BUTTON,
VIEW_ID_TRANSLATE_BUTTON,
+ // Location bar content settings icons.
+ VIEW_ID_CONTENT_SETTING_JAVASCRIPT,
+ VIEW_ID_CONTENT_SETTING_POPUP,
+
// The Bookmark Bar.
VIEW_ID_BOOKMARK_BAR,
VIEW_ID_OTHER_BOOKMARKS,
diff --git a/chrome/browser/ui/views/location_bar/content_setting_image_view.cc b/chrome/browser/ui/views/location_bar/content_setting_image_view.cc
index cfcbea2..91c90fd 100644
--- a/chrome/browser/ui/views/location_bar/content_setting_image_view.cc
+++ b/chrome/browser/ui/views/location_bar/content_setting_image_view.cc
@@ -6,10 +6,12 @@
#include <utility>
+#include "base/optional.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/themes/theme_properties.h"
#include "chrome/browser/ui/content_settings/content_setting_bubble_model.h"
#include "chrome/browser/ui/content_settings/content_setting_image_model.h"
+#include "chrome/browser/ui/view_ids.h"
#include "chrome/browser/ui/views/content_setting_bubble_contents.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/theme_provider.h"
@@ -22,6 +24,44 @@
#include "ui/views/controls/label.h"
#include "ui/views/widget/widget.h"
+namespace {
+
+base::Optional<ViewID> GetViewID(
+ ContentSettingImageModel::ImageType image_type) {
+ using ImageType = ContentSettingImageModel::ImageType;
+ switch (image_type) {
+ case ImageType::JAVASCRIPT:
+ return ViewID::VIEW_ID_CONTENT_SETTING_JAVASCRIPT;
+
+ case ImageType::POPUPS:
+ return ViewID::VIEW_ID_CONTENT_SETTING_POPUP;
+
+ case ImageType::COOKIES:
+ case ImageType::IMAGES:
+ case ImageType::PPAPI_BROKER:
+ case ImageType::PLUGINS:
+ case ImageType::GEOLOCATION:
+ case ImageType::MIXEDSCRIPT:
+ case ImageType::PROTOCOL_HANDLERS:
+ case ImageType::MEDIASTREAM:
+ case ImageType::ADS:
+ case ImageType::AUTOMATIC_DOWNLOADS:
+ case ImageType::MIDI_SYSEX:
+ case ImageType::SOUND:
+ case ImageType::FRAMEBUST:
+ case ImageType::CLIPBOARD_READ:
+ case ImageType::SENSORS:
+ return base::nullopt;
+
+ case ImageType::NUM_IMAGE_TYPES:
+ break;
+ }
+ NOTREACHED();
+ return base::nullopt;
+}
+
+} // namespace
+
ContentSettingImageView::ContentSettingImageView(
std::unique_ptr<ContentSettingImageModel> image_model,
Delegate* delegate,
@@ -33,6 +73,11 @@
DCHECK(delegate_);
SetUpForInOutAnimation();
image()->EnableCanvasFlippingForRTLUI(true);
+
+ base::Optional<ViewID> view_id =
+ GetViewID(content_setting_image_model_->image_type());
+ if (view_id)
+ SetID(*view_id);
}
ContentSettingImageView::~ContentSettingImageView() {
diff --git a/chrome/test/base/interactive_test_utils_views.cc b/chrome/test/base/interactive_test_utils_views.cc
index 3723adbe..680e864 100644
--- a/chrome/test/base/interactive_test_utils_views.cc
+++ b/chrome/test/base/interactive_test_utils_views.cc
@@ -36,12 +36,12 @@
// views::ViewObserver:
void OnViewFocused(views::View* observed_view) override {
- if (target_focused_ && run_loop_.running())
+ if (run_loop_.running() && target_focused_)
run_loop_.Quit();
}
void OnViewBlurred(views::View* observed_view) override {
- if (!target_focused_ && run_loop_.running())
+ if (run_loop_.running() && !target_focused_)
run_loop_.Quit();
}
diff --git a/chrome/test/base/ui_test_utils.cc b/chrome/test/base/ui_test_utils.cc
index 09b8418f..196d219 100644
--- a/chrome/test/base/ui_test_utils.cc
+++ b/chrome/test/base/ui_test_utils.cc
@@ -81,6 +81,12 @@
#include <windows.h>
#endif
+#if defined(TOOLKIT_VIEWS)
+#include "chrome/browser/ui/browser_window.h"
+#include "ui/views/view.h"
+#include "ui/views/widget/widget.h"
+#endif
+
using content::NavigationController;
using content::NavigationEntry;
using content::OpenURLParams;
@@ -371,6 +377,23 @@
return waiter.Wait();
}
+#if defined(TOOLKIT_VIEWS)
+void WaitForViewVisibility(Browser* browser, ViewID vid, bool visible) {
+ views::View* view = views::Widget::GetWidgetForNativeWindow(
+ browser->window()->GetNativeWindow())
+ ->GetContentsView()
+ ->GetViewByID(vid);
+ ASSERT_TRUE(view);
+ if (view->GetVisible() == visible)
+ return;
+
+ base::RunLoop run_loop;
+ auto subscription = view->AddVisibleChangedCallback(run_loop.QuitClosure());
+ run_loop.Run();
+ EXPECT_EQ(visible, view->GetVisible());
+}
+#endif
+
int FindInPage(WebContents* tab,
const base::string16& search_string,
bool forward,
diff --git a/chrome/test/base/ui_test_utils.h b/chrome/test/base/ui_test_utils.h
index 2556432..09902e1a 100644
--- a/chrome/test/base/ui_test_utils.h
+++ b/chrome/test/base/ui_test_utils.h
@@ -13,6 +13,7 @@
#include "base/macros.h"
#include "base/strings/string16.h"
+#include "chrome/browser/ui/view_ids.h"
#include "components/history/core/browser/history_service.h"
#include "content/public/browser/notification_details.h"
#include "content/public/browser/notification_observer.h"
@@ -164,6 +165,11 @@
// Blocks until an application modal dialog is shown and returns it.
app_modal::JavaScriptAppModalDialog* WaitForAppModalDialog();
+#if defined(TOOLKIT_VIEWS)
+// Blocks until the given view attains the given visibility state.
+void WaitForViewVisibility(Browser* browser, ViewID vid, bool visible);
+#endif
+
// Performs a find in the page of the specified tab. Returns the number of
// matches found. |ordinal| is an optional parameter which is set to the index
// of the current match. |selection_rect| is an optional parameter which is set
