Password manager internals page service: introduction

chrome/browser/password_manager/password_manager_internals_service.h

+// 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.
+
+#ifndef CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_INTERNALS_SERVICE_H_
+#define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_INTERNALS_SERVICE_H_
+
+#include <string>
+
+#include "components/keyed_service/core/keyed_service.h"
+#include "components/password_manager/core/browser/log_router.h"
+
+namespace content {
+class BrowserContext;
+}
+
+// Collects the logs for the password manager internals page and distributes
+// them to the open tabs with the internals page.

[Ilya Sherman, 2014/05/07 00:28:39]

nit: "the open tabs" -> "all open tabs"

[vabr (Chromium), 2014/05/07 13:42:59]

Done.

+class PasswordManagerInternalsService : public KeyedService,
+                                        public password_manager::LogRouter {
+ public:
+  // There are only two ways in which the service depends on the BrowserContext:
+  // 1) There is one service per non-incognito BrowserContext.
+  // 2) No service will be created for incognito BrowserContext.

[Ilya Sherman, 2014/05/07 00:28:39]

nit: "for incognito BrowserContext" -> "for an incognito BrowserContext" or "for
incognito BrowserContexts"

[vabr (Chromium), 2014/05/07 13:42:59]

Done.

+  // Both properties are guarantied by the BrowserContextKeyedFactory framework,
+  // so the service itself does not need the context on creation.
+  PasswordManagerInternalsService();
+  virtual ~PasswordManagerInternalsService();
+
+  // May return NULL when |context| is incognito.

[Ilya Sherman, 2014/05/07 00:28:39]

nit: "May return" -> "Returns".

[vabr (Chromium), 2014/05/07 13:42:59]

The whole function was dropped (see Colin's comment).

+  static PasswordManagerInternalsService* Get(content::BrowserContext* context);

[Ilya Sherman, 2014/05/07 00:28:39]

nit: Probably appropriate to name this GetForBrowserContext(), to match the
common pattern.

[blundell, 2014/05/07 09:05:15]

This method will have to be eliminated for the service to live in the core code
of the component.

On 2014/05/07 00:28:39, Ilya Sherman wrote:

[vabr (Chromium), 2014/05/07 13:42:59]

Agreed, but I eventually dropped the function (see below).

[vabr (Chromium), 2014/05/07 13:42:59]

Done.

+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PasswordManagerInternalsService);
+};
+
+#endif  // CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_INTERNALS_SERVICE_H_