Password manager internals page: Introduce logger in renderer

components/password_manager/core/browser/password_manager.h

 // Copyright (c) 2012 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 COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_MANAGER_H_
 #define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_MANAGER_H_
 
 #include <vector>
 
 #include "base/callback.h"
@@ skipping 81 matching lines @@
       const std::vector<autofill::PasswordForm>& forms);
 
   // Handles password forms being rendered.
   void OnPasswordFormsRendered(
       const std::vector<autofill::PasswordForm>& visible_forms);
 
   // Handles a password form being submitted.
   virtual void OnPasswordFormSubmitted(
       const autofill::PasswordForm& password_form);
 
+  PasswordManagerClient* client() const { return client_; }

[Ilya Sherman, 2014/04/09 21:10:08]

This method returns a non-const pointer, so the method should also be non-const.
 A const method should not be able to lead to any externally observable
side-effects.

[vabr (Chromium), 2014/04/10 08:51:25]

Done.
Personally, I would agree with you if the client was owned by the manager. But
that's not the case (the client actually owns the manager), so my personal
opinion is still in favour of const. But I don't feel strongly about it, hence I
removed it.

(I also split the line into three, following your note about changes in
clang-format earlier).

[Ilya Sherman, 2014/04/10 09:23:07]

You should really never use const on a method that returns a non-const pointer. 
This is actually a matter of correctness more so than it is of style.  For
example, if this method were non-const, you could write the following:

const PasswordManager password_manager = /* initialization code here */;
PasswordManager non_const_alias =
password_manager->client->GetDriver()->GetPasswordManager();
non_const_alias->ProvisionallySavePassword(/* some form */);

Tada! You've just transformed a const pointer into a non-const pointer by way of
an innocuous const method returning a non-const pointer.

In general, a good way to think about const is that calling a const method
should have no possible externally observable side-effects.  If you return a
non-const pointer from a const method, you can call a non-const method on the
returned pointer, which will tend to have externally observable side-effects,
which can lead to really surprising behavior.

[vabr (Chromium), 2014/04/10 12:15:41]

Thanks, Ilya, that's a very good point, and I agree now.

+
  private:
   enum ProvisionalSaveFailure {
     SAVING_DISABLED,
     EMPTY_PASSWORD,
     NO_MATCHING_FORM,
     MATCHING_NOT_COMPLETE,
     FORM_BLACKLISTED,
     INVALID_FORM,
     AUTOCOMPLETE_OFF,
     MAX_FAILURE_VALUE
@@ skipping 63 matching lines @@
 
   // Callbacks to be notified when a password form has been submitted.
   std::vector<PasswordSubmittedCallback> submission_callbacks_;
 
   DISALLOW_COPY_AND_ASSIGN(PasswordManager);
 };
 
 }  // namespace password_manager
 
 #endif  // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_MANAGER_H_