ShouldDisplayOverFullscreen implementation

chrome/browser/extensions/api/notifications/notifications_api.cc

Index: chrome/browser/extensions/api/notifications/notifications_api.cc
diff --git a/chrome/browser/extensions/api/notifications/notifications_api.cc b/chrome/browser/extensions/api/notifications/notifications_api.cc
index 215c9833793066d22cb29bd74b11d70567bd762f..0c3d0ef589db3651e1cf8e61d262311656262930 100644
--- a/chrome/browser/extensions/api/notifications/notifications_api.cc
+++ b/chrome/browser/extensions/api/notifications/notifications_api.cc
@@ -31,6 +31,9 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
+#include "extensions/browser/app_window/app_window.h"
+#include "extensions/browser/app_window/app_window_registry.h"
+#include "extensions/browser/app_window/native_app_window.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_system_provider.h"
 #include "extensions/browser/extensions_browser_client.h"
@@ -143,6 +146,7 @@ class NotificationsApiDelegate : public NotificationDelegate {
                            const std::string& id)
       : api_function_(api_function),
         event_router_(EventRouter::Get(profile)),
+        profile_(profile),

[dewittj, 2016/09/14 21:06:51]

Don't store profile, call GetProfile()

[bmalcolm, 2016/09/14 21:56:24]

GetProfile isn't available in NotificationsApiDelegate or NotificationDelegate -
it is available in NotificationsApiFunction, which is passing it in to the
NotificationsApiDelegate c'tor. I don't see a way to not store it unless I store
the AppWindowRegistry in the c'tor instead. 

Am I missing something?

[dewittj, 2016/09/14 23:09:46]

You're right.  One way to do it is to call api_function_->GetProfile()

But I'm beginning to worry about lifetimes.  In particular, Profile* can be
destroyed if the profile is incognito.  Holding a bare pointer could cause
crashes if the profile is destroyed.

Looks like tons of other notification delegates hold on to Profile* so it's
probably fine.

         extension_id_(extension_id),
         id_(id),
         scoped_id_(CreateScopedIdentifier(extension_id, id)) {
@@ -188,6 +192,20 @@ class NotificationsApiDelegate : public NotificationDelegate {
 
   std::string id() const override { return scoped_id_; }
 
+  // Should only display when fullscreen if this app is the source of the
+  // fullscreen window.
+  bool ShouldDisplayOverFullscreen() const override {
+    AppWindowRegistry::AppWindowList windows =
+        AppWindowRegistry::Get(profile_)->GetAppWindowsForApp(extension_id_);
+    for (const auto& window : windows) {
+      // Window must be fullscreen and visible
+      if (window->IsFullscreen() && window->GetBaseWindow()->IsActive()) {

[dewittj, 2016/09/14 21:06:51]

nit: single line if blocks should not have braces

[bmalcolm, 2016/09/14 21:56:24]

Done.

+        return true;
+      }
+    }
+    return false;
+  }
+
  private:
   ~NotificationsApiDelegate() override {}
 
@@ -222,6 +240,7 @@ class NotificationsApiDelegate : public NotificationDelegate {
   // so make sure to check for a valid pointer before use.
   EventRouter* event_router_;
 
+  Profile* profile_;
   const std::string extension_id_;
   const std::string id_;
   const std::string scoped_id_;