Password manager internals page service: introduction

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"
+
+namespace password_manager {
+
+LogRouter::LogRouter() {
+}
+
+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);
+  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 (!receivers_.might_have_observers()) {
+    accumulated_logs_.clear();

[Ilya Sherman, 2014/05/08 17:48:38]

There's a DCHECK above that asserts that this code should always be a no-op.  If
the DCHECK is correct, then there is no need to handle this case.  Are you're
worried that the DCHECKs might not be correct?

[vabr (Chromium), 2014/05/09 09:47:20]

I overlooked the DCHECK above, my bad.
Yes, I'm worried about what happens if the DCHECK assertions would not hold in a
release mode.
Although I'm fairly certain they won't, neither after this CL, nor after the
follow-up which starts using the service, there is always the risk that I'm
wrong.
The current state of the code breaks the "don't deal with a failed DCHECK" rule
no less than guarding the accumulated_logs_.append() by an if (there are some
receivers) would in ProcessLog. :(
So I'll remove the if-block, and ask you some more things in the main e-mail
body.

+    // TODO(vabr): Once the clients provide API for that, notify them if the
+    // number of receivers went from 0 to 1.
+  }
+  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.
+  }
+}
+
+}  // namespace password_manager