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

Issue 591310 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Buried. Ping if important.
Closed: Mar 2016
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 400674



Sign in to add a comment

CREDENTIAL: Android doesn't like "null" as a federation origin.

Project Member Reported by mkwst@chromium.org, Mar 2 2016

Issue description

After https://codereview.chromium.org/1723583004, we serialize `PasswordCredential` objects with a `federation_url` of "null" (as "null" is the serialization of an empty/unique origin.

Android doesn't like this. So, I suppose we need to special-case it.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 2 2016

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

commit 45bf9a3d72344f1b7a6f28e0bc1c5536339db4a8
Author: mkwst <mkwst@chromium.org>
Date: Wed Mar 02 12:56:53 2016

CREDENTIAL: Serialize 'PasswordCredential' objects with "" as the federation.

After https://codereview.chromium.org/1723583004, we serialize
`PasswordCredential` objects with a `federation_url` of "null" (as "null" is
the serialization of an empty/unique origin.

Neither Android nor Vasilii like this. This patch special-cases things to
ensure that Android (and Vasilii) remains as happy as can be.

BUG= 591310 
R=vasilii@chromium.org

Review URL: https://codereview.chromium.org/1755053002

Cr-Commit-Position: refs/heads/master@{#378734}

[modify] https://crrev.com/45bf9a3d72344f1b7a6f28e0bc1c5536339db4a8/chrome/browser/password_manager/native_backend_gnome_x.cc
[modify] https://crrev.com/45bf9a3d72344f1b7a6f28e0bc1c5536339db4a8/chrome/browser/password_manager/native_backend_gnome_x_unittest.cc
[modify] https://crrev.com/45bf9a3d72344f1b7a6f28e0bc1c5536339db4a8/chrome/browser/password_manager/native_backend_kwallet_x.cc
[modify] https://crrev.com/45bf9a3d72344f1b7a6f28e0bc1c5536339db4a8/chrome/browser/password_manager/native_backend_libsecret.cc
[modify] https://crrev.com/45bf9a3d72344f1b7a6f28e0bc1c5536339db4a8/chrome/browser/password_manager/native_backend_libsecret_unittest.cc
[modify] https://crrev.com/45bf9a3d72344f1b7a6f28e0bc1c5536339db4a8/components/password_manager/core/browser/login_database.cc

Comment 2 by mkwst@chromium.org, Mar 3 2016

Labels: Merge-Request-50
Hello friendly release managers. I'd like to merge this back to M50 to fix a bug with the serialization of password credentials into chrome sync. WDYT?

Comment 3 by tin...@google.com, Mar 3 2016

Labels: -Merge-Request-50 Merge-Approved-50 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M50 (branch: 2661)
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 3 2016

Labels: -merge-approved-50 merge-merged-2661
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0e8fd0d85b3174359835662c3dbd9f3059120729

commit 0e8fd0d85b3174359835662c3dbd9f3059120729
Author: Mike West <mkwst@google.com>
Date: Thu Mar 03 09:06:41 2016

CREDENTIAL: Serialize 'PasswordCredential' objects with "" as the federation.

After https://codereview.chromium.org/1723583004, we serialize
`PasswordCredential` objects with a `federation_url` of "null" (as "null" is
the serialization of an empty/unique origin.

Neither Android nor Vasilii like this. This patch special-cases things to
ensure that Android (and Vasilii) remains as happy as can be.

BUG= 591310 
R=vasilii@chromium.org

Review URL: https://codereview.chromium.org/1755053002

Cr-Commit-Position: refs/heads/master@{#378734}
(cherry picked from commit 45bf9a3d72344f1b7a6f28e0bc1c5536339db4a8)

Review URL: https://codereview.chromium.org/1752383004 .

Cr-Commit-Position: refs/branch-heads/2661@{#62}
Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081}

[modify] https://crrev.com/0e8fd0d85b3174359835662c3dbd9f3059120729/chrome/browser/password_manager/native_backend_gnome_x.cc
[modify] https://crrev.com/0e8fd0d85b3174359835662c3dbd9f3059120729/chrome/browser/password_manager/native_backend_gnome_x_unittest.cc
[modify] https://crrev.com/0e8fd0d85b3174359835662c3dbd9f3059120729/chrome/browser/password_manager/native_backend_kwallet_x.cc
[modify] https://crrev.com/0e8fd0d85b3174359835662c3dbd9f3059120729/chrome/browser/password_manager/native_backend_libsecret.cc
[modify] https://crrev.com/0e8fd0d85b3174359835662c3dbd9f3059120729/chrome/browser/password_manager/native_backend_libsecret_unittest.cc
[modify] https://crrev.com/0e8fd0d85b3174359835662c3dbd9f3059120729/components/password_manager/core/browser/login_database.cc

Comment 5 by mkwst@chromium.org, Mar 3 2016

Status: Fixed (was: Started)
Status: Verified (was: Fixed)
bulk verify (M50 clean up)

Sign in to add a comment