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

Issue 2598073002: MD Settings: Fix policy-related ProfileSyncService crashes. (Closed)

Created:
4 years ago by tommycli
Modified:
4 years ago
Reviewers:
Dan Beam
CC:
chromium-reviews, michaelpg+watch-md-settings_chromium.org, dbeam+watch-settings_chromium.org, stevenjb+watch-md-settings_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

MD Settings: Fix policy-related ProfileSyncService crashes. General repro steps: 1) Sign in with MD Settings. 2) Set SyncDisabled to true and wait for it to take effect. 3) MD Settings will crash shortly. BUG=651303 Committed: https://crrev.com/7c176719b12185d6ddbf531d825884dfb457e755 Cr-Commit-Position: refs/heads/master@{#440531}

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -3 lines) Patch
M chrome/browser/ui/webui/settings/people_handler.cc View 2 chunks +6 lines, -3 lines 2 comments Download

Depends on Patchset:

Messages

Total messages: 15 (9 generated)
tommycli
dbeam: PTAL, thanks!
4 years ago (2016-12-22 21:14:06 UTC) #4
Dan Beam
lgtm but maybe a test? https://codereview.chromium.org/2598073002/diff/1/chrome/browser/ui/webui/settings/people_handler.cc File chrome/browser/ui/webui/settings/people_handler.cc (right): https://codereview.chromium.org/2598073002/diff/1/chrome/browser/ui/webui/settings/people_handler.cc#newcode910 chrome/browser/ui/webui/settings/people_handler.cc:910: if (!service || service->IsFirstSetupComplete()) ...
4 years ago (2016-12-22 22:07:40 UTC) #7
tommycli
also probably not worth the test. all the instances of DCHECK(service) were just wrong, since ...
4 years ago (2016-12-22 22:46:16 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2598073002/1
4 years ago (2016-12-22 22:46:53 UTC) #10
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years ago (2016-12-22 22:51:18 UTC) #13
commit-bot: I haz the power
4 years ago (2016-12-22 22:53:35 UTC) #15
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/7c176719b12185d6ddbf531d825884dfb457e755
Cr-Commit-Position: refs/heads/master@{#440531}

Powered by Google App Engine
This is Rietveld 408576698