Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(211)

Issue 444083003: Fix crash when new avatar signin with cross account error (Closed)

Created:
6 years, 4 months ago by guohui
Modified:
6 years, 4 months ago
CC:
chromium-reviews, tim+watch_chromium.org, haitaol+watch_chromium.org, zea+watch_chromium.org, maniscalco+watch_chromium.org
Project:
chromium
Visibility:
Public.

Description

Fix crash when new avatar signin with cross account error OneClickSigninHelper::HandleCrossAccountError has an input parameter of web contents that may have been killed when the method is called, e.g. signin from the new avatar bubble. Instead this method should just use the current active tab, since it is only used to attach a tab-modal warning dialog. BUG=388607 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287973

Patch Set 1 : #

Patch Set 2 : rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -8 lines) Patch
M chrome/browser/ui/sync/one_click_signin_helper.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/sync/one_click_signin_helper.cc View 3 chunks +8 lines, -6 lines 0 comments Download
M chrome/browser/ui/webui/signin/inline_login_handler_impl.cc View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 10 (0 generated)
guohui
Hey, could you please review this CL? Thanks, Hui
6 years, 4 months ago (2014-08-06 18:48:36 UTC) #1
Roger Tawa OOO till Jul 10th
lgtm
6 years, 4 months ago (2014-08-06 19:11:31 UTC) #2
guohui
The CQ bit was checked by guohui@chromium.org
6 years, 4 months ago (2014-08-06 19:13:59 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/guohui@chromium.org/444083003/20001
6 years, 4 months ago (2014-08-06 19:15:23 UTC) #4
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win8_chromium_rel on tryserver.chromium.win ...
6 years, 4 months ago (2014-08-06 22:50:15 UTC) #5
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 4 months ago (2014-08-06 22:54:19 UTC) #6
commit-bot: I haz the power
Try jobs failed on following builders: win8_chromium_rel on tryserver.chromium.win (http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel/builds/2345)
6 years, 4 months ago (2014-08-06 22:54:21 UTC) #7
guohui
The CQ bit was checked by guohui@chromium.org
6 years, 4 months ago (2014-08-06 23:24:21 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/guohui@chromium.org/444083003/40001
6 years, 4 months ago (2014-08-06 23:27:49 UTC) #9
commit-bot: I haz the power
6 years, 4 months ago (2014-08-07 06:59:52 UTC) #10
Message was sent while issue was closed.
Change committed as 287973

Powered by Google App Engine
This is Rietveld 408576698