New issue
Advanced search Search tips

Issue 784788 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Task



Sign in to add a comment

Remove SavePasswordsCollectionViewController's deletedForms_

Project Member Reported by vabr@chromium.org, Nov 14 2017

Issue description

The comment above the definition of SavePasswordsCollectionViewController's deletedForms_ in ios/chrome/browser/ui/settings/save_passwords_collection_view_controller.mm says:

// Deletion of password being asynchronous, and the method taking a reference
// to the PasswordForm, the PasswordForm must outlive the calls to
// RemoveLogin. This vector will ensure this.

"The method" is PasswordStore::RemoveLogin, which indeed takes a reference and starts the asynchronous removal. However, it synchronously passes the reference to a base::Bind call, which makes a copy of the reference to be stored with the task scheduled asynchronously. So at the point of exit from PasswordStore::RemoveLogin, the referenced form can safely be destroyed.

Therefore, we should just get rid of SavePasswordsCollectionViewController's deletedForms_.
 

Comment 1 by ioanap@chromium.org, Nov 14 2017

Owner: ioanap@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 17 2017

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

commit 644e22dc09fbb1031462301458f6c4db5437c231
Author: Ioana Pandele <ioanap@chromium.org>
Date: Fri Nov 17 11:11:07 2017

Remove deletedForms_ from SavePasswordsCollectionViewController

PasswordStore::RemoveLogin passes the form through base::Bind which makes a copy, so this instance
doesn't need to be kept around.

Bug:  784788 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I47e02f82fa668b643d7d8c4a52907aa9b25aa404
Reviewed-on: https://chromium-review.googlesource.com/771550
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517373}
[modify] https://crrev.com/644e22dc09fbb1031462301458f6c4db5437c231/ios/chrome/browser/ui/settings/save_passwords_collection_view_controller.mm

Comment 3 by ioanap@chromium.org, Nov 17 2017

Status: Fixed (was: Assigned)

Sign in to add a comment