PushMessagingNotificationManager: extract IsTabVisible and test it.

chrome/browser/push_messaging/push_messaging_notification_manager.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/push_messaging/push_messaging_notification_manager.h"
 
 #include <bitset>
 
 #include "base/metrics/histogram_macros.h"
 #include "base/prefs/pref_service.h"
@@ skipping 123 matching lines @@
     const base::Closure& message_handled_closure,
     bool success,
     const std::vector<NotificationDatabaseData>& data) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   // TODO(johnme): Hiding an existing notification should also count as a useful
   // user-visible action done in response to a push message - but make sure that
   // sending two messages in rapid succession which show then hide a
   // 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();
     WebContents* active_web_contents = (*it)->GetActiveWebContents();
 #else
   for (chrome::BrowserIterator it; !it.done(); it.Next()) {
     Profile* profile = it->profile();
     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;
     }
 #if defined(OS_ANDROID)
   }
 #else
   }
 #endif
 
   // If more than one notification is showing for this Service Worker, close
@@ skipping 33 matching lines @@
                    weak_factory_.GetWeakPtr(), origin,
                    service_worker_registration_id, notification_shown,
                    notification_needed, message_handled_closure));
   } else {
     RecordUserVisibleStatus(
         content::PUSH_USER_VISIBLE_STATUS_NOT_REQUIRED_AND_NOT_SHOWN);
     message_handled_closure.Run();
   }
 }
 
+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 etc.

[Peter Beverloo, 2015/11/23 13:14:25]

nit: get rid of "etc" while you're at it? :)

[Michael van Ouwerkerk, 2015/11/23 13:25:09]

Done.

+  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,
     bool notification_shown,
     bool notification_needed,
     const base::Closure& message_handled_closure,
     const std::string& data,
     bool success,
     bool not_found) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ skipping 88 matching lines @@
     message_handled_closure.Run();
     return;
   }
 
   PlatformNotificationServiceImpl::GetInstance()->DisplayPersistentNotification(
       profile_, persistent_notification_id, origin, SkBitmap() /* icon */,
       notification_data);
 
   message_handled_closure.Run();
 }