New issue
Advanced search Search tips

Issue 785237 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 792852
Owner:
Closed: Mar 2018
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task

Blocking:
issue 341477
issue 789561



Sign in to add a comment

Refactor PasswordManagerPorter and PasswordUiView

Project Member Reported by cfroussios@chromium.org, Nov 15 2017

Issue description

We want to separate cross-platform code from non-cross-platform, and the export code from unrelated code
as per https://docs.google.com/document/d/1miKr2x0PTNIKgt3RQeur51ICQ66uszlYAmFXJONbG_0/edit#heading=h.shrcs072fw1n
 
Blocking: 341477
Status: Started (was: Untriaged)
Project Member

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

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

commit 0fdae7f0b507b2f397ef5afcfbae36868e878b75
Author: Christos Froussios <cfroussios@chromium.org>
Date: Wed Nov 15 16:53:32 2017

🔐 Introduce Destination inteface

It abstracts over media, to which we can send a serialised list of
exported passwords.

Bug:  785237 
Change-Id: Ib828388bb1b9b563990f16bb4e81f3fe4a68b2d9
Reviewed-on: https://chromium-review.googlesource.com/771553
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516722}
[modify] https://crrev.com/0fdae7f0b507b2f397ef5afcfbae36868e878b75/components/password_manager/core/browser/BUILD.gn
[add] https://crrev.com/0fdae7f0b507b2f397ef5afcfbae36868e878b75/components/password_manager/core/browser/export/destination.h

Project Member

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

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

commit 0126d27663187dfcecc60c35159155a836a3e738
Author: Christos Froussios <cfroussios@chromium.org>
Date: Fri Nov 17 17:18:01 2017

🔐 Introduce DestinationFileSystem

DestinationFileSystem is the desktop implemenation of Destination, which
sends the data to the file system.

Bug:  785237 
Change-Id: I034fab562eafde2240dc75743cb2ebbc6f7ec9ac
Reviewed-on: https://chromium-review.googlesource.com/776596
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517442}
[modify] https://crrev.com/0126d27663187dfcecc60c35159155a836a3e738/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/0126d27663187dfcecc60c35159155a836a3e738/chrome/browser/ui/passwords/destination_file_system.cc
[add] https://crrev.com/0126d27663187dfcecc60c35159155a836a3e738/chrome/browser/ui/passwords/destination_file_system.h
[add] https://crrev.com/0126d27663187dfcecc60c35159155a836a3e738/chrome/browser/ui/passwords/destination_file_system_unittest.cc
[modify] https://crrev.com/0126d27663187dfcecc60c35159155a836a3e738/chrome/browser/ui/passwords/password_manager_porter.cc
[modify] https://crrev.com/0126d27663187dfcecc60c35159155a836a3e738/chrome/test/BUILD.gn
[modify] https://crrev.com/0126d27663187dfcecc60c35159155a836a3e738/components/password_manager/core/browser/export/destination.h

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 28 2017

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

commit bb1822f27dbb0e614e976f65dc7dfc8bd115dc39
Author: Christos Froussios <cfroussios@chromium.org>
Date: Tue Nov 28 15:57:04 2017

🔐 Introduce PasswordManagerExporter

This class is responsible for cross-platform logic for exporting
passwords.

Bug:  785237 
Change-Id: Idc3f77f66ba6211324d82415bb9d42328823b7e5
Reviewed-on: https://chromium-review.googlesource.com/782722
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519698}
[modify] https://crrev.com/bb1822f27dbb0e614e976f65dc7dfc8bd115dc39/chrome/browser/ui/passwords/destination_file_system.cc
[modify] https://crrev.com/bb1822f27dbb0e614e976f65dc7dfc8bd115dc39/chrome/browser/ui/passwords/destination_file_system.h
[modify] https://crrev.com/bb1822f27dbb0e614e976f65dc7dfc8bd115dc39/chrome/browser/ui/passwords/password_manager_porter.cc
[modify] https://crrev.com/bb1822f27dbb0e614e976f65dc7dfc8bd115dc39/chrome/browser/ui/passwords/password_manager_porter.h
[modify] https://crrev.com/bb1822f27dbb0e614e976f65dc7dfc8bd115dc39/chrome/browser/ui/passwords/password_manager_porter_unittest.cc
[modify] https://crrev.com/bb1822f27dbb0e614e976f65dc7dfc8bd115dc39/components/password_manager/core/browser/BUILD.gn
[modify] https://crrev.com/bb1822f27dbb0e614e976f65dc7dfc8bd115dc39/components/password_manager/core/browser/export/destination.h
[add] https://crrev.com/bb1822f27dbb0e614e976f65dc7dfc8bd115dc39/components/password_manager/core/browser/export/password_manager_exporter.cc
[add] https://crrev.com/bb1822f27dbb0e614e976f65dc7dfc8bd115dc39/components/password_manager/core/browser/export/password_manager_exporter.h
[add] https://crrev.com/bb1822f27dbb0e614e976f65dc7dfc8bd115dc39/components/password_manager/core/browser/export/password_manager_exporter_unittest.cc

Blocking: 789561
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 31 2018

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

commit 56bde75e233d70c2516519d361246d539bf84bc6
Author: Christos Froussios <cfroussios@chromium.org>
Date: Wed Jan 31 13:45:31 2018

[Password Manager] Preparing the passwords for export also serialises them

I also changed the timing on the IN_PROGRESS update to occur as soon as
the destination is known. If we happen to still be serialising, that is an
implementation detail that should not be affect the UI.

Bug:  785237 
Change-Id: Ie9466840b1e4bf11ddfaa4c69045326ab3b11fe3
Reviewed-on: https://chromium-review.googlesource.com/789330
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533264}
[modify] https://crrev.com/56bde75e233d70c2516519d361246d539bf84bc6/components/password_manager/core/browser/export/password_manager_exporter.cc
[modify] https://crrev.com/56bde75e233d70c2516519d361246d539bf84bc6/components/password_manager/core/browser/export/password_manager_exporter.h
[modify] https://crrev.com/56bde75e233d70c2516519d361246d539bf84bc6/components/password_manager/core/browser/export/password_manager_exporter_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 9 2018

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

commit 3121b3349e8846fab825b5fbb6d0b6bf2cfad870
Author: Christos Froussios <cfroussios@chromium.org>
Date: Fri Feb 09 18:28:47 2018

[Password Manager] Start serialising passwords for export before file selection

Reading and serialising can take a non-trivial amount of time. If we do
this while the user is selecting the file destination, we decrease the
chance that there will be a delay after the file is selected.

Bug:  785237 , 789561 
Change-Id: Ifae55091733e0310345385cbdb05023246e2ebff
Reviewed-on: https://chromium-review.googlesource.com/911792
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535757}
[modify] https://crrev.com/3121b3349e8846fab825b5fbb6d0b6bf2cfad870/chrome/browser/ui/passwords/password_manager_porter.cc

Mergedinto: 792852
Status: Duplicate (was: Started)
We will not need cross-platform in the way described here. The abstractions introduced here should be revisited and possibly removed for simplicity.

Sign in to add a comment