Password manager internals page service: wiring it in

components/password_manager/core/browser/log_router.cc

 // Copyright 2014 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 "components/password_manager/core/browser/log_router.h"
 
 #include "base/stl_util.h"
 #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() {
 }
 
 void LogRouter::ProcessLog(const std::string& text) {
   // This may not be called when there are no receivers (i.e., the router is
   // 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));
 }
 
 bool LogRouter::RegisterClient(PasswordManagerClient* client) {
   DCHECK(client);
   clients_.AddObserver(client);
   return receivers_.might_have_observers();
 }
 
 void LogRouter::UnregisterClient(PasswordManagerClient* client) {
   DCHECK(clients_.HasObserver(client));
   clients_.RemoveObserver(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_;
 }
 
 void LogRouter::UnregisterReceiver(PasswordManagerLogger* receiver) {
   DCHECK(receivers_.HasObserver(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));
   }
 }
 
 }  // namespace password_manager