Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2623033006:
[Re-landing] Migrate external protocol prefs from local state to profiles (Closed)
|
Created:
3 years, 11 months ago by ramyasharma Modified:
3 years, 10 months ago CC:
chromium-reviews, jdonnelly+watch_chromium.org, sky Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Re-landing] Migrate external protocol prefs from local state to profiles.
Moves external protocol handler state from local state to per-profile.
This ensures that external protocol settings maybe distinct per-profile
and simplifies the implementation of editing UI.
The local state data is migrated to the currently active profile and then
deleted. This may result in some loss for people who use multiple
profiles; However, they will be presented with an external protocol
dialog the next time they try and open a formerly remembered scheme.
It also resets the state of all schemes which have been permanently
blocked by users (and are not part of the pre-populated set of schemes).
These schemes will prompt the user the next time they are accessed. We
do this because:
a) there is currently no UI to edit external handlers, so users cannot
recover from permanently silencing a dialog.
b) local state is per Chrome instance, meaning that users have to wipe
their entire Chrome data dir (or edit the local state JSON file) to recover.
BUG=457254, 62799
TBR=sky@chromium.org
Review-Url: https://codereview.chromium.org/2623033006
Cr-Commit-Position: refs/heads/master@{#446959}
Committed: https://chromium.googlesource.com/chromium/src/+/03dfa11bd46784b21cc75aa8cef21cb81435722c
Re-landing as the test that caused this cl to be reverted has been disabled due to being flaky : https://bugs.chromium.org/p/chromium/issues/detail?id=686803
Review-Url: https://codereview.chromium.org/2623033006
Cr-Commit-Position: refs/heads/master@{#448213}
Committed: https://chromium.googlesource.com/chromium/src/+/2c618e171bf9c6fbe4ffa58e59d67c826491893d
Patch Set 1 : a #
Total comments: 35
Patch Set 2 : a #
Total comments: 2
Patch Set 3 : a #Patch Set 4 : a #
Total comments: 4
Patch Set 5 : a #Patch Set 6 : rebase #Messages
Total messages: 119 (89 generated)
Description was changed from ========== Initial changes to move external protocol handler state from local state to profile local state to profile changes initial changes to move external protocol handler state from local state to profile BUG= ========== to ========== Initial changes to move external protocol handler state from local state to profile BUG= ==========
Description was changed from ========== Initial changes to move external protocol handler state from local state to profile BUG= ========== to ========== ==========
Description was changed from ========== ========== to ========== BUG=457254 ==========
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
Patchset #1 (id:60001) has been deleted
Patchset #1 (id:80001) has been deleted
Patchset #1 (id:100001) has been deleted
Patchset #1 (id:120001) has been deleted
Patchset #1 (id:140001) has been deleted
Patchset #1 (id:160001) has been deleted
ramyasharma@chromium.org changed reviewers: + dominickn@chromium.org
Thanks! I mostly have style related comments in this pass. Also: please change the subject to something that fits in 80 characters. You can move the reset stuff into the CL description (wrapped to 80 characters), like: Migrate external protocol prefs from local state to profiles. This CL does <blah blah> The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; however, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the prepopulated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... File chrome/browser/external_protocol/external_protocol_handler.cc (right): https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:152: // TODO(pkasting): This kind of thing should go in the preferences on the Nit: you can remove this TODO, because you're doing it. :) In its place, you can say: // Check if there are any prefs in the local state. If there are, wipe them, and migrate the prefs to the profile. // TODO(ramyasharma) remove the migration in M61. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:154: PrefService* localPref = g_browser_process->local_state(); Nit: variables use underscore_naming, so call this local_state_prefs https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:155: PrefService* profilePref = profile->GetPrefs(); Call this profile_prefs https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:157: DictionaryPrefUpdate update_excluded_schemas( Call this local_state_schemas, since we're not actually going to update it any more? https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:169: if (it.value().GetAsBoolean(&is_blocked) && !is_blocked) { Nit: one-line if statements don't need braces https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:176: // Prepolutate the default states each time. sp. "Prepopulate" https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:180: if (update_excluded_schemas_profile->GetBoolean(scheme, &should_block)) { Nit: don't add the braces here, one line if statements don't need them https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:193: // TODO(pkasting): This kind of thing should go in the preferences on the Nit: remove this TODO (you're doing it) https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:196: PrefService* profilePref = profile->GetPrefs(); local_state_prefs and profile_prefs https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:204: is_profile_state What do you think about migrating here as well? My feeling is that every time SetBlockState would be called, we would have initially called GetBlockState and it would have been migrated already right? https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:208: is_profile_state Nit: indentation. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:211: :update_excluded_schemas->SetBoolean(scheme, (state == BLOCK)); Nit: space after : https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:233: Profile* profile = NULL; Use nullptr instead of NULL (this is a very old file so it references NULL everywhere) https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:234: if (web_contents) { // Maybe NULL during testing. Nit: no braces around a one-line else. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... File chrome/browser/external_protocol/external_protocol_handler_unittest.cc (right): https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler_unittest.cc:130: TestingBrowserProcess::GetGlobal()->SetLocalState(NULL); Nit: use nullptr instead of NULL https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler_unittest.cc:236: ASSERT_EQ(ExternalProtocolHandler::UNKNOWN, block_state); Can you assert that the local_state_ pref is empty after this runs? And that the profile pref has the right data in it (i.e. nothing?) https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler_unittest.cc:246: ASSERT_EQ(ExternalProtocolHandler::DONT_BLOCK, block_state); Ditto
Description was changed from ========== BUG=457254 ========== to ========== Moves external protocol handler state from local state to per profile, and also reset the users stuck in Do nothing state, when moving these settings to per-profile BUG=457254 ==========
Description was changed from ========== Moves external protocol handler state from local state to per profile, and also reset the users stuck in Do nothing state, when moving these settings to per-profile BUG=457254 ========== to ========== Moves external protocol handler state from local state to per profile, and also resets the users stuck in Do nothing state, when moving these settings to per-profile The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; however, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover BUG=457254 ==========
Description was changed from ========== Moves external protocol handler state from local state to per profile, and also resets the users stuck in Do nothing state, when moving these settings to per-profile The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; however, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover BUG=457254 ========== to ========== Moves external protocol handler state from local state to per profile, and also resets the users stuck in Do nothing state, when moving these settings to per-profile The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; however, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover BUG=457254,62799 ==========
https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... File chrome/browser/external_protocol/external_protocol_handler.cc (right): https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:152: // TODO(pkasting): This kind of thing should go in the preferences on the On 2017/01/19 07:01:22, dominickn wrote: > Nit: you can remove this TODO, because you're doing it. :) In its place, you can > say: > > // Check if there are any prefs in the local state. If there are, wipe them, and > migrate the prefs to the profile. > // TODO(ramyasharma) remove the migration in M61. Done. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:154: PrefService* localPref = g_browser_process->local_state(); On 2017/01/19 07:01:22, dominickn wrote: > Nit: variables use underscore_naming, so call this local_state_prefs Done. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:155: PrefService* profilePref = profile->GetPrefs(); On 2017/01/19 07:01:22, dominickn wrote: > Call this profile_prefs Done. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:157: DictionaryPrefUpdate update_excluded_schemas( On 2017/01/19 07:01:22, dominickn wrote: > Call this local_state_schemas, since we're not actually going to update it any > more? Done. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:169: if (it.value().GetAsBoolean(&is_blocked) && !is_blocked) { On 2017/01/19 07:01:22, dominickn wrote: > Nit: one-line if statements don't need braces Done. I was wondering about that, why is it not? Until now I have been following the rule that always add braces even if its one line, that ensures no bugs are introduced later because of missing braces. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:176: // Prepolutate the default states each time. On 2017/01/19 07:01:22, dominickn wrote: > sp. "Prepopulate" Oops, done. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:180: if (update_excluded_schemas_profile->GetBoolean(scheme, &should_block)) { On 2017/01/19 07:01:22, dominickn wrote: > Nit: don't add the braces here, one line if statements don't need them Done. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:193: // TODO(pkasting): This kind of thing should go in the preferences on the On 2017/01/19 07:01:22, dominickn wrote: > Nit: remove this TODO (you're doing it) Done. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:196: PrefService* profilePref = profile->GetPrefs(); On 2017/01/19 07:01:22, dominickn wrote: > local_state_prefs and profile_prefs Done. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:204: is_profile_state On 2017/01/19 07:01:22, dominickn wrote: > What do you think about migrating here as well? My feeling is that every time > SetBlockState would be called, we would have initially called GetBlockState and > it would have been migrated already right? Yes, ideally that's how it is. Removed code that updates local state, and using only profile state to update these prefs. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:208: is_profile_state On 2017/01/19 07:01:22, dominickn wrote: > Nit: indentation. Done. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:211: :update_excluded_schemas->SetBoolean(scheme, (state == BLOCK)); On 2017/01/19 07:01:22, dominickn wrote: > Nit: space after : Done. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:233: Profile* profile = NULL; On 2017/01/19 07:01:22, dominickn wrote: > Use nullptr instead of NULL (this is a very old file so it references NULL > everywhere) Done. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:234: if (web_contents) { // Maybe NULL during testing. On 2017/01/19 07:01:22, dominickn wrote: > Nit: no braces around a one-line else. Done. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... File chrome/browser/external_protocol/external_protocol_handler_unittest.cc (right): https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler_unittest.cc:130: TestingBrowserProcess::GetGlobal()->SetLocalState(NULL); On 2017/01/19 07:01:22, dominickn wrote: > Nit: use nullptr instead of NULL Done. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler_unittest.cc:236: ASSERT_EQ(ExternalProtocolHandler::UNKNOWN, block_state); On 2017/01/19 07:01:22, dominickn wrote: > Can you assert that the local_state_ pref is empty after this runs? And that the > profile pref has the right data in it (i.e. nothing?) Added a check to local state, to ensure it has nothing. Profile pref is not expected to be empty though. https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler_unittest.cc:246: ASSERT_EQ(ExternalProtocolHandler::DONT_BLOCK, block_state); On 2017/01/19 07:01:22, dominickn wrote: > Ditto Done.
Patchset #2 (id:200001) has been deleted
lgtm - thanks Ramya! On Monday we'll figure out the right OWNERs to send this to. Minor nit: Make the first line of the description the same as the subject (that's what goes into the git commit message when the patch lands). https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... File chrome/browser/external_protocol/external_protocol_handler.cc (right): https://codereview.chromium.org/2623033006/diff/180001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler.cc:169: if (it.value().GetAsBoolean(&is_blocked) && !is_blocked) { On 2017/01/20 04:04:45, ramyasharma wrote: > On 2017/01/19 07:01:22, dominickn wrote: > > Nit: one-line if statements don't need braces > > Done. I was wondering about that, why is it not? Until now I have been following > the rule that always add braces even if its one line, that ensures no bugs are > introduced later because of missing braces. I used to follow that rule too, but this style is much more prevalent in Chromium, so for consistency I usually suggest doing it this way. I think it's just "one of the opinions" :)
Description was changed from ========== Moves external protocol handler state from local state to per profile, and also resets the users stuck in Do nothing state, when moving these settings to per-profile The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; however, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover BUG=457254,62799 ========== to ========== Migrate external protocol prefs from local state to profiles Moves external protocol handler state from local state to per profile, and also resets the users stuck in Do nothing state, when moving these settings to per-profile. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; however, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover BUG=457254,62799 ==========
ramyasharma@chromium.org changed reviewers: + anthonyvd@chromium.org, meacer@chromium.org, msw@chromium.org, tapted@chromium.org
Description was changed from ========== Migrate external protocol prefs from local state to profiles Moves external protocol handler state from local state to per profile, and also resets the users stuck in Do nothing state, when moving these settings to per-profile. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; however, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover BUG=457254,62799 ========== to ========== Migrate external protocol prefs from local state to profiles Moves external protocol handler state from local state to per profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. This CL also resets the users stuck in Do nothing state, when moving these settings to per-profile. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; however, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover BUG=457254,62799 ==========
Description was changed from ========== Migrate external protocol prefs from local state to profiles Moves external protocol handler state from local state to per profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. This CL also resets the users stuck in Do nothing state, when moving these settings to per-profile. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; however, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover BUG=457254,62799 ========== to ========== Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 ==========
Description was changed from ========== Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 ========== to ========== Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 ==========
Description was changed from ========== Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 ========== to ========== Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 ==========
Adding meacer@ for chrome/browser/external_protocol/* tapted@ for API change in chrome/browser/ui/cocoa/external_protocol_dialog.mm anthonyvd@ for chrome/browser/profiles/profile.cc msw@ for API change in chrome/browser/ui/external_protocol_dialog_delegate.cc I plan to merge this change into M57.
external_protocol_dialog.mm lgtm with the following (I didn't look closely at the rest). https://codereview.chromium.org/2623033006/diff/220001/chrome/browser/ui/coco... File chrome/browser/ui/cocoa/external_protocol_dialog.mm (right): https://codereview.chromium.org/2623033006/diff/220001/chrome/browser/ui/coco... chrome/browser/ui/cocoa/external_protocol_dialog.mm:127: content::WebContents* web_contents = nit: move this up to line ~110 and reuse
Thanks tapted@. https://codereview.chromium.org/2623033006/diff/220001/chrome/browser/ui/coco... File chrome/browser/ui/cocoa/external_protocol_dialog.mm (right): https://codereview.chromium.org/2623033006/diff/220001/chrome/browser/ui/coco... chrome/browser/ui/cocoa/external_protocol_dialog.mm:127: content::WebContents* web_contents = On 2017/01/23 05:07:54, tapted wrote: > nit: move this up to line ~110 and reuse Done.
The CQ bit was checked by ramyasharma@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by ramyasharma@chromium.org
external_protocol_dialog_delegate.cc lgtm
profile.cc lgtm
The CQ bit was checked by ramyasharma@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by ramyasharma@chromium.org
The CQ bit was checked by ramyasharma@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #4 (id:260001) has been deleted
The CQ bit was checked by ramyasharma@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
Patchset #4 (id:280001) has been deleted
The CQ bit was checked by ramyasharma@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Adding davidben@chromium.org for chrome/browser/prerender/prerender_test_utils.cc
ramyasharma@chromium.org changed reviewers: + davidben@chromium.org
davidben@chromium.org: Please review changes in chrome/browser/prerender/prerender_test_utils.cc
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Patchset #4 (id:300001) has been deleted
The CQ bit was checked by ramyasharma@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
alexmos@chromium.org Please review changes in chrome/browser/chrome_site_per_process_browsertest.cc
ramyasharma@chromium.org changed reviewers: + alexmos@chromium.org
alexmos@chromium.org: Please review changes in chrome/browser/chrome_site_per_process_browsertest.cc
prerender lgtm
chrome_site_per_process_browsertest.cc LGTM
Lgtm https://codereview.chromium.org/2623033006/diff/320001/chrome/browser/externa... File chrome/browser/external_protocol/external_protocol_handler_unittest.cc (right): https://codereview.chromium.org/2623033006/diff/320001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler_unittest.cc:213: ExternalProtocolHandler::GetBlockState("tel", profile_.get()); nit: Indentation seems off here (2 more spaces) and in the next test cases. Maybe run git cl format? https://codereview.chromium.org/2623033006/diff/320001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler_unittest.cc:239: TestGetBlockStateLocalBlockStateCopiedAndResetOnProfilePref) { nit: Indentation, here and below.
Thanks Mustafa. https://codereview.chromium.org/2623033006/diff/320001/chrome/browser/externa... File chrome/browser/external_protocol/external_protocol_handler_unittest.cc (right): https://codereview.chromium.org/2623033006/diff/320001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler_unittest.cc:213: ExternalProtocolHandler::GetBlockState("tel", profile_.get()); On 2017/01/25 22:15:07, Mustafa Emre Acer wrote: > nit: Indentation seems off here (2 more spaces) and in the next test cases. > Maybe run git cl format? Thanks Done. https://codereview.chromium.org/2623033006/diff/320001/chrome/browser/externa... chrome/browser/external_protocol/external_protocol_handler_unittest.cc:239: TestGetBlockStateLocalBlockStateCopiedAndResetOnProfilePref) { On 2017/01/25 22:15:07, Mustafa Emre Acer wrote: > nit: Indentation, here and below. Done.
Description was changed from ========== Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 ========== to ========== Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 TBR=sky@chromium.org ==========
Description was changed from ========== Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 TBR=sky@chromium.org ========== to ========== Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 ==========
TBR sky@ for chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.cc (minor API change)
The CQ bit was checked by ramyasharma@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dominickn@chromium.org, tapted@chromium.org, msw@chromium.org, anthonyvd@chromium.org, davidben@chromium.org, alexmos@chromium.org, meacer@chromium.org Link to the patchset: https://codereview.chromium.org/2623033006/#ps340001 (title: "a")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
Patchset #5 (id:340001) has been deleted
The CQ bit was checked by ramyasharma@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #5 (id:360001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by ramyasharma@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dominickn@chromium.org, tapted@chromium.org, anthonyvd@chromium.org, davidben@chromium.org, msw@chromium.org, alexmos@chromium.org, meacer@chromium.org Link to the patchset: https://codereview.chromium.org/2623033006/#ps380001 (title: "a")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Description was changed from ========== Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 ========== to ========== Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 TBR=sky@chromium.org ==========
The CQ bit was checked by ramyasharma@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 380001, "attempt_start_ts": 1485751615842720,
"parent_rev": "f8c3441865c7e2e559e21ee8e37093b10af0ad26", "commit_rev":
"03dfa11bd46784b21cc75aa8cef21cb81435722c"}
Message was sent while issue was closed.
Description was changed from ========== Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 TBR=sky@chromium.org ========== to ========== Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 TBR=sky@chromium.org Review-Url: https://codereview.chromium.org/2623033006 Cr-Commit-Position: refs/heads/master@{#446959} Committed: https://chromium.googlesource.com/chromium/src/+/03dfa11bd46784b21cc75aa8cef2... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:380001) as https://chromium.googlesource.com/chromium/src/+/03dfa11bd46784b21cc75aa8cef2...
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:380001) has been created in https://codereview.chromium.org/2660333004/ by tsergeant@chromium.org. The reason for reverting is: Speculative revert for crbug.com/686803: This CL appears to be causing the test MTPDeviceDelegateImplWinTest.GalleryNameMTP to fail on Win7 Tests (dbg)(1). See findit results: https://findit-for-me.appspot.com/waterfall/build-failure?url=https://build.c... and first failed build: https://luci-milo.appspot.com/buildbot/chromium.win/Win7%20Tests%20(dbg)(1)/5....
Message was sent while issue was closed.
Description was changed from ========== Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 TBR=sky@chromium.org Review-Url: https://codereview.chromium.org/2623033006 Cr-Commit-Position: refs/heads/master@{#446959} Committed: https://chromium.googlesource.com/chromium/src/+/03dfa11bd46784b21cc75aa8cef2... ========== to ========== [Re-landing] Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 TBR=sky@chromium.org Review-Url: https://codereview.chromium.org/2623033006 Cr-Commit-Position: refs/heads/master@{#446959} Committed: https://chromium.googlesource.com/chromium/src/+/03dfa11bd46784b21cc75aa8cef2... Re-landing as the test that caused this cl to be reverted has been disabled due to being flaky : https://bugs.chromium.org/p/chromium/issues/detail?id=686803 ==========
The CQ bit was checked by ramyasharma@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by ramyasharma@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
Patchset #6 (id:400001) has been deleted
The CQ bit was checked by ramyasharma@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
Patchset #6 (id:420001) has been deleted
The CQ bit was checked by ramyasharma@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by ramyasharma@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dominickn@chromium.org, tapted@chromium.org, anthonyvd@chromium.org, davidben@chromium.org, msw@chromium.org, alexmos@chromium.org, meacer@chromium.org Link to the patchset: https://codereview.chromium.org/2623033006/#ps440001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by ramyasharma@chromium.org
The CQ bit was checked by ramyasharma@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 440001, "attempt_start_ts": 1486353499521530,
"parent_rev": "28428173446e3a4ae4cf214144b77514839cbba1", "commit_rev":
"2c618e171bf9c6fbe4ffa58e59d67c826491893d"}
Message was sent while issue was closed.
Description was changed from ========== [Re-landing] Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 TBR=sky@chromium.org Review-Url: https://codereview.chromium.org/2623033006 Cr-Commit-Position: refs/heads/master@{#446959} Committed: https://chromium.googlesource.com/chromium/src/+/03dfa11bd46784b21cc75aa8cef2... Re-landing as the test that caused this cl to be reverted has been disabled due to being flaky : https://bugs.chromium.org/p/chromium/issues/detail?id=686803 ========== to ========== [Re-landing] Migrate external protocol prefs from local state to profiles. Moves external protocol handler state from local state to per-profile. This ensures that external protocol settings maybe distinct per-profile and simplifies the implementation of editing UI. The local state data is migrated to the currently active profile and then deleted. This may result in some loss for people who use multiple profiles; However, they will be presented with an external protocol dialog the next time they try and open a formerly remembered scheme. It also resets the state of all schemes which have been permanently blocked by users (and are not part of the pre-populated set of schemes). These schemes will prompt the user the next time they are accessed. We do this because: a) there is currently no UI to edit external handlers, so users cannot recover from permanently silencing a dialog. b) local state is per Chrome instance, meaning that users have to wipe their entire Chrome data dir (or edit the local state JSON file) to recover. BUG=457254,62799 TBR=sky@chromium.org Review-Url: https://codereview.chromium.org/2623033006 Cr-Commit-Position: refs/heads/master@{#446959} Committed: https://chromium.googlesource.com/chromium/src/+/03dfa11bd46784b21cc75aa8cef2... Re-landing as the test that caused this cl to be reverted has been disabled due to being flaky : https://bugs.chromium.org/p/chromium/issues/detail?id=686803 Review-Url: https://codereview.chromium.org/2623033006 Cr-Commit-Position: refs/heads/master@{#448213} Committed: https://chromium.googlesource.com/chromium/src/+/2c618e171bf9c6fbe4ffa58e59d6... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:440001) as https://chromium.googlesource.com/chromium/src/+/2c618e171bf9c6fbe4ffa58e59d6... |
