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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 64313005: Fix crash when locking a profile. (Closed)

Created:
7 years, 1 month ago by noms (inactive)
Modified:
7 years, 1 month ago
CC:
Peter Kasting, chromium-reviews, tfarina, stevenjb+watch_chromium.org, davemoore+watch_chromium.org, oshima+watch_chromium.org, nkostylev+watch_chromium.org
Visibility:
Public.

Description

Fix crash when locking a profile. The crash is happening because navigating to a URL triggers the signin header helper code. Since locking shouldn't actually do a Gaia logout, I'm removing all the code related to that. NOTRY-ing because it's been a week of "extension "RANDR" missing on display ":9" bot errors. BUG=NONE TBR=pkasting@chromium.org NOTRY=TRUE Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=235801 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=235989

Patch Set 1 #

Total comments: 4

Patch Set 2 : roger nits #

Patch Set 3 : fix return type on cros #

Total comments: 2

Patch Set 4 : pkasting nits #

Patch Set 5 : s/BeginSignOut/LockProfile/g #

Patch Set 6 : rebase #

Patch Set 7 : fix compile #

Patch Set 8 : fix flaky test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+28 lines, -137 lines) Patch
M chrome/browser/chromeos/profiles/avatar_menu_actions_chromeos.h View 1 2 3 4 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/profiles/avatar_menu_actions_chromeos.cc View 1 2 3 4 1 chunk +0 lines, -9 lines 0 comments Download
M chrome/browser/extensions/api/principals_private/principals_private_api.cc View 1 2 3 4 2 chunks +2 lines, -5 lines 0 comments Download
M chrome/browser/profiles/avatar_menu.h View 1 2 3 4 1 chunk +0 lines, -6 lines 0 comments Download
M chrome/browser/profiles/avatar_menu.cc View 1 2 3 4 1 chunk +0 lines, -8 lines 0 comments Download
M chrome/browser/profiles/avatar_menu_actions.h View 1 2 3 4 1 chunk +0 lines, -6 lines 0 comments Download
M chrome/browser/profiles/avatar_menu_actions_desktop.h View 1 2 3 4 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/profiles/avatar_menu_actions_desktop.cc View 1 2 3 4 2 chunks +0 lines, -90 lines 0 comments Download
M chrome/browser/profiles/profile_list_desktop_browsertest.cc View 1 2 3 4 5 6 7 2 chunks +8 lines, -4 lines 0 comments Download
M chrome/browser/profiles/profile_window.h View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/profiles/profile_window.cc View 1 2 3 4 2 chunks +12 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/avatar_menu_bubble_view.cc View 1 2 3 4 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/ui/views/new_avatar_menu_button_browsertest.cc View 1 2 3 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/ui/views/profile_chooser_view.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 39 (0 generated)
Roger Tawa OOO till Jul 10th
lgtm, a couple of nits below. https://codereview.chromium.org/64313005/diff/1/chrome/browser/chromeos/profiles/avatar_menu_actions_chromeos.cc File chrome/browser/chromeos/profiles/avatar_menu_actions_chromeos.cc (right): https://codereview.chromium.org/64313005/diff/1/chrome/browser/chromeos/profiles/avatar_menu_actions_chromeos.cc#newcode48 chrome/browser/chromeos/profiles/avatar_menu_actions_chromeos.cc:48: content::WebContents* AvatarMenuActionsChromeOS::BeginSignOut() { ...
7 years, 1 month ago (2013-11-12 15:27:48 UTC) #1
Roger Tawa OOO till Jul 10th
lgtm, a couple of nits below. https://codereview.chromium.org/64313005/diff/1/chrome/browser/chromeos/profiles/avatar_menu_actions_chromeos.cc File chrome/browser/chromeos/profiles/avatar_menu_actions_chromeos.cc (right): https://codereview.chromium.org/64313005/diff/1/chrome/browser/chromeos/profiles/avatar_menu_actions_chromeos.cc#newcode48 chrome/browser/chromeos/profiles/avatar_menu_actions_chromeos.cc:48: content::WebContents* AvatarMenuActionsChromeOS::BeginSignOut() { ...
7 years, 1 month ago (2013-11-12 15:27:48 UTC) #2
noms (inactive)
https://codereview.chromium.org/64313005/diff/1/chrome/browser/chromeos/profiles/avatar_menu_actions_chromeos.cc File chrome/browser/chromeos/profiles/avatar_menu_actions_chromeos.cc (right): https://codereview.chromium.org/64313005/diff/1/chrome/browser/chromeos/profiles/avatar_menu_actions_chromeos.cc#newcode48 chrome/browser/chromeos/profiles/avatar_menu_actions_chromeos.cc:48: content::WebContents* AvatarMenuActionsChromeOS::BeginSignOut() { Oops. Missed one. Done! On 2013/11/12 ...
7 years, 1 month ago (2013-11-12 16:47:26 UTC) #3
noms (inactive)
Hi, Since locking a profile is not meant to do a Gaia logout anymore, I'm ...
7 years, 1 month ago (2013-11-12 16:54:07 UTC) #4
Nikita (slow)
lgtm
7 years, 1 month ago (2013-11-12 17:27:50 UTC) #5
rpetterson
profiles lgtm
7 years, 1 month ago (2013-11-12 19:29:03 UTC) #6
noms (inactive)
Thanks!
7 years, 1 month ago (2013-11-12 19:37:54 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noms@chromium.org/64313005/190001
7 years, 1 month ago (2013-11-12 19:40:23 UTC) #8
Peter Kasting
LGTM https://codereview.chromium.org/64313005/diff/190001/chrome/browser/ui/views/new_avatar_menu_button_browsertest.cc File chrome/browser/ui/views/new_avatar_menu_button_browsertest.cc (right): https://codereview.chromium.org/64313005/diff/190001/chrome/browser/ui/views/new_avatar_menu_button_browsertest.cc#newcode113 chrome/browser/ui/views/new_avatar_menu_button_browsertest.cc:113: ui::MouseEvent mouse_ev(ui::ET_MOUSE_RELEASED, gfx::Point(), gfx::Point(), 0); This variable is ...
7 years, 1 month ago (2013-11-12 19:49:20 UTC) #9
noms (inactive)
https://codereview.chromium.org/64313005/diff/190001/chrome/browser/ui/views/new_avatar_menu_button_browsertest.cc File chrome/browser/ui/views/new_avatar_menu_button_browsertest.cc (right): https://codereview.chromium.org/64313005/diff/190001/chrome/browser/ui/views/new_avatar_menu_button_browsertest.cc#newcode113 chrome/browser/ui/views/new_avatar_menu_button_browsertest.cc:113: ui::MouseEvent mouse_ev(ui::ET_MOUSE_RELEASED, gfx::Point(), gfx::Point(), 0); On 2013/11/12 19:49:21, Peter ...
7 years, 1 month ago (2013-11-12 19:55:53 UTC) #10
commit-bot: I haz the power
Failed to trigger a try job on win_rel HTTP Error 400: Bad Request
7 years, 1 month ago (2013-11-12 19:58:37 UTC) #11
noms (inactive)
Hi Rachel, I've changed the api a little bit -- I renamed the BeginSignOut function ...
7 years, 1 month ago (2013-11-12 20:35:21 UTC) #12
noms (inactive)
+ hui for rubber stamp on c/b/extensions/api/
7 years, 1 month ago (2013-11-12 20:36:33 UTC) #13
rpetterson
On 2013/11/12 20:35:21, Monica Dinculescu wrote: > Hi Rachel, > > I've changed the api ...
7 years, 1 month ago (2013-11-12 22:10:36 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noms@chromium.org/64313005/590001
7 years, 1 month ago (2013-11-13 15:32:21 UTC) #15
guohui
lgtm
7 years, 1 month ago (2013-11-13 16:55:11 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noms@chromium.org/64313005/720001
7 years, 1 month ago (2013-11-13 18:32:57 UTC) #17
commit-bot: I haz the power
Retried try job too often on linux_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&number=189370
7 years, 1 month ago (2013-11-13 23:59:21 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noms@chromium.org/64313005/720001
7 years, 1 month ago (2013-11-14 00:13:32 UTC) #19
commit-bot: I haz the power
Retried try job too often on linux_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&number=189615
7 years, 1 month ago (2013-11-14 04:43:34 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noms@chromium.org/64313005/720001
7 years, 1 month ago (2013-11-14 14:10:25 UTC) #21
commit-bot: I haz the power
Retried try job too often on linux_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&number=189867
7 years, 1 month ago (2013-11-14 17:07:15 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noms@chromium.org/64313005/720001
7 years, 1 month ago (2013-11-14 17:42:35 UTC) #23
commit-bot: I haz the power
Retried try job too often on linux_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&number=190049
7 years, 1 month ago (2013-11-14 20:50:58 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noms@chromium.org/64313005/720001
7 years, 1 month ago (2013-11-15 15:57:20 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noms@chromium.org/64313005/720001
7 years, 1 month ago (2013-11-15 22:02:04 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noms@chromium.org/64313005/720001
7 years, 1 month ago (2013-11-16 00:30:23 UTC) #27
commit-bot: I haz the power
Retried try job too often on linux_aura for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura&number=98254
7 years, 1 month ago (2013-11-16 05:14:20 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noms@chromium.org/64313005/720001
7 years, 1 month ago (2013-11-17 14:26:44 UTC) #29
commit-bot: I haz the power
Retried try job too often on linux_aura for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura&number=98313
7 years, 1 month ago (2013-11-17 17:46:56 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noms@chromium.org/64313005/720001
7 years, 1 month ago (2013-11-17 21:34:55 UTC) #31
commit-bot: I haz the power
Retried try job too often on linux_aura for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura&number=98331
7 years, 1 month ago (2013-11-18 00:54:44 UTC) #32
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noms@chromium.org/64313005/720001
7 years, 1 month ago (2013-11-18 15:51:27 UTC) #33
commit-bot: I haz the power
Retried try job too often on linux_aura for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura&number=98489
7 years, 1 month ago (2013-11-18 19:51:53 UTC) #34
noms (inactive)
No-trying this CL because it's been a sad week of "extension "RANDR" missing on display ...
7 years, 1 month ago (2013-11-18 19:58:15 UTC) #35
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noms@chromium.org/64313005/720001
7 years, 1 month ago (2013-11-18 19:58:57 UTC) #36
commit-bot: I haz the power
Change committed as 235801
7 years, 1 month ago (2013-11-18 20:01:24 UTC) #37
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noms@chromium.org/64313005/1830001
7 years, 1 month ago (2013-11-19 14:18:01 UTC) #38
commit-bot: I haz the power
7 years, 1 month ago (2013-11-19 14:18:47 UTC) #39
Message was sent while issue was closed. 
Change committed as 235989

Powered by Google App Engine
This is Rietveld 408576698