Password manager internals page service: wiring it in

components/password_manager/core/browser/log_router.cc

Index: components/password_manager/core/browser/log_router.cc
diff --git a/components/password_manager/core/browser/log_router.cc b/components/password_manager/core/browser/log_router.cc
index c8719a88152a177450b5d9d135d90ca178b79e2a..064718dd1c8f3a359af4e3c3146184333c316e71 100644
--- a/components/password_manager/core/browser/log_router.cc
+++ b/components/password_manager/core/browser/log_router.cc
@@ -8,9 +8,28 @@
 #include "components/password_manager/core/browser/password_manager_client.h"
 #include "components/password_manager/core/browser/password_manager_logger.h"
 
+#if !defined(NDEBUG)
+#include "base/memory/scoped_ptr.h"
+#endif
+
 namespace password_manager {
 
+namespace {
+
+#if !defined(NDEBUG)
+// Built to the image of base::DefaultDeleter, TrueSetter sets a bool* pointer's
+// target to true, rather than deleting it.
+struct TrueSetter {
+  inline void operator()(bool* ptr) const { *ptr = true; }
+};
+#endif
+
+}  // namespace
+
 LogRouter::LogRouter() {
+#if !defined(NDEBUG)
+  can_use_receivers_ = true;
+#endif
 }
 
 LogRouter::~LogRouter() {
@@ -21,6 +40,7 @@ void LogRouter::ProcessLog(const std::string& text) {
   // inactive), because in that case the logs cannot be displayed.
   DCHECK(receivers_.might_have_observers());
   accumulated_logs_.append(text);
+  DCHECK(can_use_receivers_);

[Ilya Sherman, 2014/05/13 04:32:18]

Hmm, this seems like a lot of work just for the sake of a DCHECK.  I'd omit all
the extra complexity, and just trust that you've verified this correctly.  I'd
expect that tools like ASan would catch any errors anyway.  In general, DCHECKs
are nice; but if they start requiring a bunch of work to maintain, it's probably
not worth the effort unless that work generalizes to many different callsites
(e.g. as ThreadChecker does).

[vabr (Chromium), 2014/05/13 09:27:11]

Done.
I agree. I double-checked that the current code would not trigger the DCHECK. As
long as the comment at PasswordManagerClient::NotifyCanUseLogRouter is honoured,
that should not be a problem.

   FOR_EACH_OBSERVER(
       PasswordManagerLogger, receivers_, LogSavePasswordProgress(text));
 }
@@ -39,9 +59,18 @@ void LogRouter::UnregisterClient(PasswordManagerClient* client) {
 std::string LogRouter::RegisterReceiver(PasswordManagerLogger* receiver) {
   DCHECK(receiver);
   DCHECK(accumulated_logs_.empty() || receivers_.might_have_observers());
+#if !defined(NDEBUG)
+  // Make sure the receivers are not dereferenced until we return. This might
+  // get called from the constructor of |receiver|.
+  DCHECK(can_use_receivers_);
+  can_use_receivers_ = false;
+  scoped_ptr<bool, TrueSetter> lock(&can_use_receivers_);
+#endif
 
-  // TODO(vabr): Once the clients provide API for that, notify them if the
-  // number of receivers went from 0 to 1.
+  if (!receivers_.might_have_observers()) {
+    FOR_EACH_OBSERVER(
+        PasswordManagerClient, clients_, NotifyCanUseLogRouter(true));
+  }
   receivers_.AddObserver(receiver);
   return accumulated_logs_;
 }
@@ -51,8 +80,8 @@ void LogRouter::UnregisterReceiver(PasswordManagerLogger* receiver) {
   receivers_.RemoveObserver(receiver);
   if (!receivers_.might_have_observers()) {
     accumulated_logs_.clear();
-    // TODO(vabr): Once the clients provide API for that, notify them that the
-    // number of receivers went from 1 to 0.
+    FOR_EACH_OBSERVER(
+        PasswordManagerClient, clients_, NotifyCanUseLogRouter(false));
   }
 }