New issue
Advanced search Search tips

Issue 920973 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 16
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

people_handler.cc still opens chrome:chrome-signin in edge cases

Project Member Reported by droger@chromium.org, Jan 11

Issue description

chrome:chrome-signin is deprecated and unsupported, yet it is still used in some places.

when force_new_tab is true, something wrong happens:

https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/settings/people_handler.cc?rcl=cfb41cfbc1a49990cdb64d2407c0fd107ac9a276&l=376
 
Components: Services>SignIn
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 16 (6 days ago)

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

commit faec37cf9d92d037166c74d7adb0d4eb6a2ff6ab
Author: David Roger <droger@chromium.org>
Date: Wed Jan 16 09:06:22 2019

[signin] Remove embedded signin flow from people_handler

The emedded signin URL is used when there is no browser opened. This
case can no longer really happen in practice, unless the browser is
somehow closed as the same time as the signin button is pressed.
It is fine to do noting in this case.

See  https://crbug.com/385546  for context about this code.
The original use case (the settings app) no longer exists.

Bug:  920973 
Change-Id: I42e67df716361c7cb46fbf168d277e3b7168b01b
Reviewed-on: https://chromium-review.googlesource.com/c/1411596
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623163}
[modify] https://crrev.com/faec37cf9d92d037166c74d7adb0d4eb6a2ff6ab/chrome/browser/ui/webui/settings/people_handler.cc

Comment 3 by droger@chromium.org, Jan 16 (6 days ago)

Status: Fixed (was: Untriaged)
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 16 (6 days ago)

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

commit 200d78d850f5f04322c02bd399e204c3b98b8c2d
Author: David Roger <droger@chromium.org>
Date: Wed Jan 16 16:17:16 2019

[signin] Remove more uses of chrome://chrome-signin on desktop

Since Dice Milestone 2 is now enabled by default on desktop, support
for the embedded signin flow can be removed.

Bug:  920973 
Change-Id: I0d808f94cb03644ed0a212f5b00285924ca4bd2c
Reviewed-on: https://chromium-review.googlesource.com/c/1411696
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623252}
[modify] https://crrev.com/200d78d850f5f04322c02bd399e204c3b98b8c2d/chrome/browser/signin/signin_ui_util.cc
[modify] https://crrev.com/200d78d850f5f04322c02bd399e204c3b98b8c2d/chrome/browser/ui/chrome_pages.cc
[modify] https://crrev.com/200d78d850f5f04322c02bd399e204c3b98b8c2d/chrome/browser/ui/signin_view_controller.cc
[modify] https://crrev.com/200d78d850f5f04322c02bd399e204c3b98b8c2d/chrome/browser/ui/signin_view_controller.h

Sign in to add a comment