Remove obsolete TODO(vabr) from passwords

There are a few TODO(vabr) in the code. Some of them are about old
code-health improvements which proved to be unimportant enough to
consider, those are being removed in this CL. A few are still relevant
and have an associated bug filed, those are redirected to that bug
instead.

Bug: 908813, 397083, 415449, 543085
Change-Id: If506009ce54016f8a1d9680a90b6516b399af4aa
Reviewed-on: https://chromium-review.googlesource.com/c/1352186Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611292}

chrome/browser/password_manager/chrome_password_manager_client_unittest.cc

@@ -75,8 +75,8 @@ using testing::Return;
 using testing::_;
 
 namespace {
-// TODO(vabr): Get rid of the mocked client in the client's own test, see
-// http://crbug.com/474577.
+// TODO(crbug.com/474577): Get rid of the mocked client in the client's own
+// test.
 class MockChromePasswordManagerClient : public ChromePasswordManagerClient {
  public:
   MOCK_CONST_METHOD0(GetMainFrameCertStatus, net::CertStatus());

chrome/browser/password_manager/native_backend_gnome_x_unittest.cc

@@ -835,8 +835,6 @@ TEST_F(NativeBackendGnomeTest, PSLUpdatingStrictUpdateLogin) {
 }
 
 TEST_F(NativeBackendGnomeTest, PSLUpdatingStrictAddLogin) {
-  // TODO(vabr): if AddLogin becomes no longer valid for existing logins, then
-  // just delete this test.
   CheckPSLUpdate(UPDATE_BY_ADDLOGIN);
 }
 

chrome/browser/password_manager/native_backend_libsecret_unittest.cc

@@ -707,8 +707,6 @@ TEST_F(NativeBackendLibsecretTest, PSLUpdatingStrictUpdateLogin) {
 }
 
 TEST_F(NativeBackendLibsecretTest, PSLUpdatingStrictAddLogin) {
-  // TODO(vabr): if AddLogin becomes no longer valid for existing logins, then
-  // just delete this test.
   CheckPSLUpdate(UPDATE_BY_ADDLOGIN);
 }
 

chrome/browser/password_manager/password_manager_internals_service_unittest.cc

@@ -52,7 +52,6 @@ TEST_F(PasswordManagerInternalsServiceTest, ServiceActiveNonIncognito) {
   ASSERT_TRUE(service);
   EXPECT_EQ(std::string(), service->RegisterReceiver(&receiver));
 
-  // TODO(vabr): Use a MockPasswordManagerClient to detect activity changes.
   EXPECT_CALL(receiver, LogSavePasswordProgress(kTestText)).Times(1);
   service->ProcessLog(kTestText);
 

components/autofill/content/renderer/password_autofill_agent.cc

@@ -735,7 +735,7 @@ void PasswordAutofillAgent::PasswordValueGatekeeper::ShowValue(
 
 bool PasswordAutofillAgent::TextDidChangeInTextField(
     const WebInputElement& element) {
-  // TODO(vabr): Get a mutable argument instead. http://crbug.com/397083
+  // TODO(crbug.com/415449): Do this through const WebInputElement.
   WebInputElement mutable_element = element;  // We need a non-const.
   mutable_element.SetAutofillState(WebAutofillState::kNotFilled);
 
@@ -754,7 +754,7 @@ void PasswordAutofillAgent::DidEndTextFieldEditing() {
 
 void PasswordAutofillAgent::UpdateStateForTextChange(
     const WebInputElement& element) {
-  // TODO(vabr): Get a mutable argument instead. http://crbug.com/397083
+  // TODO(crbug.com/415449): Do this through const WebInputElement.
   WebInputElement mutable_element = element;  // We need a non-const.
 
   if (element.IsTextField()) {

components/autofill/core/common/save_password_progress_logger.h

@@ -28,11 +28,6 @@ struct PasswordForm;
 //
 // To use this class, the method SendLog needs to be overriden to send the logs
 // for display as appropriate.
-//
-// TODO(vabr): Logically, this class belongs to the password_manager component.
-// But the PasswordAutofillAgent needs to use it, so until that agent is in a
-// third component, shared by autofill and password_manager, this helper needs
-// to stay in autofill as well.
 class SavePasswordProgressLogger {
  public:
   // IDs of strings allowed in the logs: for security reasons, we only pass the

components/password_manager/core/browser/password_manager.cc

@@ -597,10 +597,9 @@ void PasswordManager::AddObserverAndDeliverCredentials(
     const PasswordForm& observed_form) {
   observers_.AddObserver(observer);
 
+  // The observers are responsible for filtering notifications by the observer
+  // signon_realm. Each notification is broadcasted to every observer.
   observer->set_signon_realm(observed_form.signon_realm);
-  // TODO(vabr): Even though the observers do the realm check, this mechanism
-  // will still result in every observer being notified about every form. We
-  // could perhaps do better by registering an observer call-back instead.
 
   std::vector<PasswordForm> observed_forms;
   observed_forms.push_back(observed_form);

components/password_manager/core/browser/sync_credentials_filter.cc

@@ -30,9 +30,9 @@ bool LastLoadWasTransactionalReauthPage(const GURL& last_load_url) {
       GaiaUrls::GetInstance()->gaia_url().GetOrigin())
     return false;
 
-  // TODO(vabr): GAIA stops using the "rart" URL param, and instead includes a
-  // hidden form field with name "rart". http://crbug.com/543085
-  // "rart" is the transactional reauth paramter.
+  // TODO(crbug.com/543085): GAIA stops using the "rart" URL param, and instead
+  // includes a hidden form field with name "rart". "rart" is the transactional
+  // reauth paramter.
   std::string ignored_value;
   return net::GetValueForKeyInQuery(last_load_url, "rart", &ignored_value);
 }

components/password_manager/core/browser/sync_credentials_filter.h

@@ -34,8 +34,6 @@ class SyncCredentialsFilter : public CredentialsFilter {
   // commited entry URL for a check against GAIA reauth site. Uses the factory
   // functions repeatedly to get the sync service and signin manager to pass
   // them to sync_util methods.
-  // TODO(vabr): Could we safely just get a pointer to the services for the
-  // lifetime of the filter?
   SyncCredentialsFilter(
       const PasswordManagerClient* client,
       SyncServiceFactoryFunction sync_service_factory_function,