New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 795234 link

Starred by 1 user
Status: Fixed
Owner:
█ vabr@chromium.org
hobby only
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

[Password Manager] Exclude |old_primary_key| from the |FormSaver::Save| list of arguments

Project Member Reported by kolos@chromium.org, Dec 15 2017 
|old_primary_key| is never assigned or used when new credential is saved. It also looks a bit confusing. 

Vaclav: preliminary assign to you.

Comment 1 by vabr@chromium.org, Jan 9 2018

Labels: Hotlist-TechnicalDebt
Summary: [Password Manager] Exclude |old_primary_key| from the |FormSaver::Save| list of arguments (was: [Password Manager] Exclude |old_primary_key| from the |FormSaver:Save| list of arguments)
Indeed, |old_primary_key| should be dropped from |FormSaver::Save| (but not from |FormSaver::Update|. Not sure how it got there. I'll do this soon.
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 10 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dda29a1dfad913b3faf377ba6569ef5234df9b25

commit dda29a1dfad913b3faf377ba6569ef5234df9b25
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Jan 10 15:12:23 2018

Remove old_primary_key from FormSaver::Save

The old_primary_key argument is only ever used for updating a
password, not saving it. It appears to be in the signature of
FormSaver::Save by mistake and is never used (always null).

This CL removes that argument.

Bug:  795234 
Change-Id: Id6db5a8c1cb5237842e8c2fd4494296d2e50ef9c
Reviewed-on: https://chromium-review.googlesource.com/859790
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528313}
[modify] https://crrev.com/dda29a1dfad913b3faf377ba6569ef5234df9b25/components/password_manager/core/browser/form_saver.h
[modify] https://crrev.com/dda29a1dfad913b3faf377ba6569ef5234df9b25/components/password_manager/core/browser/form_saver_impl.cc
[modify] https://crrev.com/dda29a1dfad913b3faf377ba6569ef5234df9b25/components/password_manager/core/browser/form_saver_impl.h
[modify] https://crrev.com/dda29a1dfad913b3faf377ba6569ef5234df9b25/components/password_manager/core/browser/form_saver_impl_unittest.cc
[modify] https://crrev.com/dda29a1dfad913b3faf377ba6569ef5234df9b25/components/password_manager/core/browser/password_form_manager.cc
[modify] https://crrev.com/dda29a1dfad913b3faf377ba6569ef5234df9b25/components/password_manager/core/browser/password_form_manager_unittest.cc
[modify] https://crrev.com/dda29a1dfad913b3faf377ba6569ef5234df9b25/components/password_manager/core/browser/stub_form_saver.h

Comment 3 by vabr@chromium.org, Jan 10 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment