Add KeepAliveStateObserver, add the Restart option

chrome/browser/lifetime/keep_alive_registry.cc

 // Copyright 2016 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/lifetime/keep_alive_registry.h"
 
 #include "chrome/browser/lifetime/application_lifetime.h"
+#include "chrome/browser/lifetime/keep_alive_state_observer.h"
 #include "chrome/browser/lifetime/keep_alive_types.h"
 
 ////////////////////////////////////////////////////////////////////////////////
 // Public methods
 
 // static
 KeepAliveRegistry* KeepAliveRegistry::GetInstance() {
   return base::Singleton<KeepAliveRegistry>::get();
 }
 
-bool KeepAliveRegistry::WillKeepAlive() const {
+bool KeepAliveRegistry::IsKeepingAlive() const {
   return registered_count_ > 0;
 }
 
+bool KeepAliveRegistry::IsRestartAllowed() const {
+  return registered_count_ == restart_allowed_count_;
+}
+
+void KeepAliveRegistry::AddObserver(KeepAliveStateObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void KeepAliveRegistry::RemoveObserver(KeepAliveStateObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Private methods
 
-KeepAliveRegistry::KeepAliveRegistry() : registered_count_(0) {}
+KeepAliveRegistry::KeepAliveRegistry()
+    : registered_count_(0), restart_allowed_count_(0) {}
 
 KeepAliveRegistry::~KeepAliveRegistry() {
   DCHECK_EQ(0, registered_count_);
   DCHECK_EQ(0u, registered_keep_alives_.size());
+  DCHECK_EQ(0, restart_allowed_count_);
 }
 
-void KeepAliveRegistry::Register(KeepAliveOrigin origin) {
-  if (!WillKeepAlive())
-    chrome::IncrementKeepAliveCount();
+void KeepAliveRegistry::Register(KeepAliveOrigin origin,
+                                 KeepAliveRestartOption restart) {
+  bool old_keeping_alive = IsKeepingAlive();

[sky, 2016/02/26 22:13:25]

49-50 and 58-65 are duplicated in UnRegister. How about a helper class whose
constructor has 49-50 and destructor 58-65?

[dgn, 2016/03/07 17:17:14]

I considered something similar before (see up to patchset 4 for example:
https://codereview.chromium.org/1725883002/diff2/60001:100001/chrome/browser/...)

An issue is that we might want to revert the notification order between adding
and removing, in case there are dependencies. Here the KeepAlive and
RestartAllowed notifications are swapped for example. I made quite granular
notifying and state reading methods to allow for that while keeping it simple
and concise in Register/Unregister. So you prefer the approach from patch 4?

[sky, 2016/03/09 18:29:35]

Ok. Keep what you have here.

[dgn, 2016/03/09 18:37:04]

Acknowledged.

+  bool old_restart_allowed = IsRestartAllowed();
 
   ++registered_keep_alives_[origin];
   ++registered_count_;
+
+  if (restart == KeepAliveRestartOption::ENABLED)
+    ++restart_allowed_count_;
+
+  bool new_keeping_alive = IsKeepingAlive();
+  bool new_restart_allowed = IsRestartAllowed();
+
+  if (new_keeping_alive != old_keeping_alive)
+    OnKeepingAliveChanged(new_keeping_alive);
+
+  if (new_restart_allowed != old_restart_allowed)
+    OnRestartAllowedChanged(new_restart_allowed);
+
+  DVLOG(1) << "New state of the KeepAliveRegistry: " << *this;
 }
 
-void KeepAliveRegistry::Unregister(KeepAliveOrigin origin) {
+void KeepAliveRegistry::Unregister(KeepAliveOrigin origin,
+                                   KeepAliveRestartOption restart) {
+ bool old_keeping_alive = IsKeepingAlive();
+ bool old_restart_allowed = IsRestartAllowed();
+
   --registered_count_;
   DCHECK_GE(registered_count_, 0);
 
   int new_count = --registered_keep_alives_[origin];
   DCHECK_GE(registered_keep_alives_[origin], 0);
   if (new_count == 0)
     registered_keep_alives_.erase(origin);
 
-  if (!WillKeepAlive())
+  if (restart == KeepAliveRestartOption::ENABLED)
+    --restart_allowed_count_;
+
+  bool new_keeping_alive = IsKeepingAlive();
+  bool new_restart_allowed = IsRestartAllowed();
+
+  if (new_restart_allowed != old_restart_allowed)
+    OnRestartAllowedChanged(new_restart_allowed);
+
+  if (new_keeping_alive != old_keeping_alive)
+    OnKeepingAliveChanged(new_keeping_alive);
+
+  DVLOG(1) << "New state of the KeepAliveRegistry: " << *this;
+}
+
+
+void KeepAliveRegistry::OnKeepingAliveChanged(bool new_keeping_alive) {
+  if (new_keeping_alive) {
+    DVLOG(1) << "KeepAliveRegistry is now keeping the browser alive.";
+    chrome::IncrementKeepAliveCount();
+  } else {
+    DVLOG(1) << "KeepAliveRegistry stopped keeping the browser alive.";
     chrome::DecrementKeepAliveCount();
+  }
 }
+void KeepAliveRegistry::OnRestartAllowedChanged(bool new_restart_allowed) {
+  if (new_restart_allowed) {
+    DVLOG(1) << "Notifying KeepAliveStateObservers: Restart Allowed";
+    FOR_EACH_OBSERVER(KeepAliveStateObserver, observers_, OnRestartAllowed());
+  } else {
+    DVLOG(1) << "Notifying KeepAliveStateObservers: Restart Forbidden";
+    FOR_EACH_OBSERVER(KeepAliveStateObserver, observers_, OnRestartForbidden());
+  }
+}
+
+#ifndef NDEBUG
+std::ostream& operator<<(std::ostream& out, const KeepAliveRegistry& registry) {
+  out << "{KeepingAlive=" << registry.IsKeepingAlive() << ", RestartAllowed="
+      << registry.IsRestartAllowed() << ", KeepAlives=[";
+  for (auto counts_per_origin_it : registry.registered_keep_alives_) {
+    if (counts_per_origin_it != *registry.registered_keep_alives_.begin())
+      out << ", ";
+    out << counts_per_origin_it.first << " (" << counts_per_origin_it.second
+        << ")";
+  }
+  out << "]}";
+  return out;
+}
+#endif  // ndef NDEBUG