Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2585)

Unified Diff: chrome/browser/push_messaging/push_messaging_notification_manager.cc

Issue 1467573003: PushMessagingNotificationManager: extract IsTabVisible and test it. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address peter's comments. Created 5 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/push_messaging/push_messaging_notification_manager.cc
diff --git a/chrome/browser/push_messaging/push_messaging_notification_manager.cc b/chrome/browser/push_messaging/push_messaging_notification_manager.cc
index e96bb796d33fb2ec5e6777e3f00dc45313bd937d..88e0aa171051b3941b0d2f57fe62bb168f01e8be 100644
--- a/chrome/browser/push_messaging/push_messaging_notification_manager.cc
+++ b/chrome/browser/push_messaging/push_messaging_notification_manager.cc
@@ -141,11 +141,9 @@ void PushMessagingNotificationManager::DidGetNotificationsFromDatabase(
// notification doesn't count.
int notification_count = success ? data.size() : 0;
bool notification_shown = notification_count > 0;
-
bool notification_needed = true;
// Sites with a currently visible tab don't need to show notifications.
-
#if defined(OS_ANDROID)
for (auto it = TabModelList::begin(); it != TabModelList::end(); ++it) {
Profile* profile = (*it)->GetProfile();
@@ -156,26 +154,7 @@ void PushMessagingNotificationManager::DidGetNotificationsFromDatabase(
WebContents* active_web_contents =
it->tab_strip_model()->GetActiveWebContents();
#endif
- if (!active_web_contents || !active_web_contents->GetMainFrame())
- continue;
-
- // Don't leak information from other profiles.
- if (profile != profile_)
- continue;
-
- // Ignore minimized windows etc.
- switch (active_web_contents->GetMainFrame()->GetVisibilityState()) {
- case blink::WebPageVisibilityStateHidden:
- case blink::WebPageVisibilityStatePrerender:
- continue;
- case blink::WebPageVisibilityStateVisible:
- break;
- }
-
- // Use the visible URL since that's the one the user is aware of (and it
- // doesn't matter whether the page loaded successfully).
- const GURL& active_url = active_web_contents->GetVisibleURL();
- if (origin == active_url.GetOrigin()) {
+ if (IsTabVisible(profile, active_web_contents, origin)) {
notification_needed = false;
break;
}
@@ -229,6 +208,31 @@ void PushMessagingNotificationManager::DidGetNotificationsFromDatabase(
}
}
+bool PushMessagingNotificationManager::IsTabVisible(
+ Profile* profile,
+ WebContents* active_web_contents,
+ const GURL& origin) {
+ if (!active_web_contents || !active_web_contents->GetMainFrame())
+ return false;
+
+ // Don't leak information from other profiles.
+ if (profile != profile_)
+ return false;
+
+ // Ignore minimized windows.
+ switch (active_web_contents->GetMainFrame()->GetVisibilityState()) {
+ case blink::WebPageVisibilityStateHidden:
+ case blink::WebPageVisibilityStatePrerender:
+ return false;
+ case blink::WebPageVisibilityStateVisible:
+ break;
+ }
+
+ // Use the visible URL since that's the one the user is aware of (and it
+ // doesn't matter whether the page loaded successfully).
+ return origin == active_web_contents->GetVisibleURL().GetOrigin();
+}
+
void PushMessagingNotificationManager::DidGetNotificationsShownAndNeeded(
const GURL& origin,
int64_t service_worker_registration_id,

Powered by Google App Engine
This is Rietveld 408576698