Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2519953004:
Profile deletion logic refactored. (Closed)
DescriptionUnify profile deletion logic across platforms.
Profile deletion have quite fuzzy logic while deleting active profile:
Sole profile deletion will open another window with newly created
profile. If there is only one browser running, and there is another
profile - on MacOS we will open browser with it, on other platforms
browser will be closed. And finaly if there two browsers with different
profiles running, and total three or more profiles - on MacOS new
browser can or can not be opened.
What we shall do:
1. If deletion happend with not active profile and not under guest
profile - silently delete it.
2. If an active profile is pending for deletion - find another browser
running, and use its profile as fallback.
3. If there no another browser running - load any existing profile.
4. If there no non-legacy-supervised profiles left - create a new one.
5. Mark existed/loaded/created profile as active and pass it to the
callback.
From user perspective logic become much more clear, if we take some
action which kill current browser - we either switch to another open
browser, or open a new one with appropriate profile. No more platform
specific behaviour or special rules dependant on total profile count.
What changed:
* Removed Mac-specific case when it can open a new browser window while
there is browser with another profile running.
* On non-Mac platforms deleting active profile with no other browser
running will not terminate browser process, instead a new browser
window will be oppened with existed or newly created profile.
BUG=
R=anthonyvd@chromium.org, bauerb@chromium.org
Committed: https://crrev.com/198d711e1e047b20d235c430a697f1b39a651b08
Cr-Commit-Position: refs/heads/master@{#434463}
Patch Set 1 #
Total comments: 2
Patch Set 2 : review fixes #
Total comments: 1
Patch Set 3 : Comment typo fixed. #Messages
Total messages: 42 (17 generated)
This changes existing behavior, right? Specifically, if a) we're on Linux or Windows, b) there is more than one profile, and c) the currently active profile is deleted, we will now open a new window with the remaining profile, whereas we used to quit, correct? Could you explain in the CL description why we want to do that? https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... File chrome/browser/profiles/profile_manager.cc (right): https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... chrome/browser/profiles/profile_manager.cc:1326: for (auto* browser : *BrowserList::GetInstance()) { Use the real type instead of auto? https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... chrome/browser/profiles/profile_manager.cc:1361: #if !defined(OS_CHROMEOS) && !defined(OS_ANDROID) As an aside, this is already inside of an !defined(OS_ANDROID) block :)
On 2016/11/22 14:16:45, Bernhard Bauer wrote: > This changes existing behavior, right? Specifically, if > > a) we're on Linux or Windows, > b) there is more than one profile, and > c) the currently active profile is deleted, > > we will now open a new window with the remaining profile, whereas we used to > quit, correct? Could you explain in the CL description why we want to do that? > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > File chrome/browser/profiles/profile_manager.cc (right): > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > chrome/browser/profiles/profile_manager.cc:1326: for (auto* browser : > *BrowserList::GetInstance()) { > Use the real type instead of auto? > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > chrome/browser/profiles/profile_manager.cc:1361: #if !defined(OS_CHROMEOS) && > !defined(OS_ANDROID) > As an aside, this is already inside of an !defined(OS_ANDROID) block :) From user perspective logic become much more clear, if we take some action which kill current browser - we either switch to another open browser, or open a new one with appropriate profile. No more platform specific behaviour or special rules dependant on total profile count.
Description was changed from ========== Profile deletion logic refactored. Profile deletion have quite fuzzy logic while deleting active profile: Sole profile deletion will open another window with newly created profile. If there is only one browser running, and there is another profile - on MacOS we will open browser with it, on other platforms browser will be closed. And finaly if there two browsers with different profiles running, and total three or more profiles - on MacOS new browser can or can not be opened. What we shall do: 1. If deletion happend with not active profile and not under guest profile - silently delete it. 2. If an active profile is pending for deletion - find another browser running, and use its profile as fallback. 3. If there no another browser running - load any existing profile. 4. If there no non-legacy-supervised profiles left - create a new one. 5. Mark existed/loaded/created profile as active and pass it to the callback. BUG= R=anthonyvd@chromium.org, bauerb@chromium.org ========== to ========== Profile deletion logic refactored. Profile deletion have quite fuzzy logic while deleting active profile: Sole profile deletion will open another window with newly created profile. If there is only one browser running, and there is another profile - on MacOS we will open browser with it, on other platforms browser will be closed. And finaly if there two browsers with different profiles running, and total three or more profiles - on MacOS new browser can or can not be opened. What we shall do: 1. If deletion happend with not active profile and not under guest profile - silently delete it. 2. If an active profile is pending for deletion - find another browser running, and use its profile as fallback. 3. If there no another browser running - load any existing profile. 4. If there no non-legacy-supervised profiles left - create a new one. 5. Mark existed/loaded/created profile as active and pass it to the callback. From user perspective logic become much more clear, if we take some action which kill current browser - we either switch to another open browser, or open a new one with appropriate profile. No more platform specific behaviour or special rules dependant on total profile count. BUG= R=anthonyvd@chromium.org, bauerb@chromium.org ==========
On 2016/11/22 14:16:45, Bernhard Bauer wrote: > This changes existing behavior, right? Specifically, if > > a) we're on Linux or Windows, > b) there is more than one profile, and > c) the currently active profile is deleted, > > we will now open a new window with the remaining profile, whereas we used to > quit, correct? Could you explain in the CL description why we want to do that? > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > File chrome/browser/profiles/profile_manager.cc (right): > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > chrome/browser/profiles/profile_manager.cc:1326: for (auto* browser : > *BrowserList::GetInstance()) { > Use the real type instead of auto? > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > chrome/browser/profiles/profile_manager.cc:1361: #if !defined(OS_CHROMEOS) && > !defined(OS_ANDROID) > As an aside, this is already inside of an !defined(OS_ANDROID) block :) done.
On 2016/11/22 14:34:15, palar wrote: > On 2016/11/22 14:16:45, Bernhard Bauer wrote: > > This changes existing behavior, right? Specifically, if > > > > a) we're on Linux or Windows, > > b) there is more than one profile, and > > c) the currently active profile is deleted, > > > > we will now open a new window with the remaining profile, whereas we used to > > quit, correct? Could you explain in the CL description why we want to do that? > > > > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > > File chrome/browser/profiles/profile_manager.cc (right): > > > > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > > chrome/browser/profiles/profile_manager.cc:1326: for (auto* browser : > > *BrowserList::GetInstance()) { > > Use the real type instead of auto? > > > > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > > chrome/browser/profiles/profile_manager.cc:1361: #if !defined(OS_CHROMEOS) && > > !defined(OS_ANDROID) > > As an aside, this is already inside of an !defined(OS_ANDROID) block :) > > From user perspective logic become much more clear, if we take some > action which kill current browser - we either switch to another open > browser, or open a new one with appropriate profile. No more platform > specific behaviour or special rules dependant on total profile count. OK, I think that makes sense. Can we make the change in behavior more explicit in the CL though? Something like "Remove Mac special case when deleting the active profile" as the first line. Also, +ewald@ (Identity PM) as FYI.
Description was changed from ========== Profile deletion logic refactored. Profile deletion have quite fuzzy logic while deleting active profile: Sole profile deletion will open another window with newly created profile. If there is only one browser running, and there is another profile - on MacOS we will open browser with it, on other platforms browser will be closed. And finaly if there two browsers with different profiles running, and total three or more profiles - on MacOS new browser can or can not be opened. What we shall do: 1. If deletion happend with not active profile and not under guest profile - silently delete it. 2. If an active profile is pending for deletion - find another browser running, and use its profile as fallback. 3. If there no another browser running - load any existing profile. 4. If there no non-legacy-supervised profiles left - create a new one. 5. Mark existed/loaded/created profile as active and pass it to the callback. From user perspective logic become much more clear, if we take some action which kill current browser - we either switch to another open browser, or open a new one with appropriate profile. No more platform specific behaviour or special rules dependant on total profile count. BUG= R=anthonyvd@chromium.org, bauerb@chromium.org ========== to ========== Profile deletion logic refactored. Profile deletion have quite fuzzy logic while deleting active profile: Sole profile deletion will open another window with newly created profile. If there is only one browser running, and there is another profile - on MacOS we will open browser with it, on other platforms browser will be closed. And finaly if there two browsers with different profiles running, and total three or more profiles - on MacOS new browser can or can not be opened. What we shall do: 1. If deletion happend with not active profile and not under guest profile - silently delete it. 2. If an active profile is pending for deletion - find another browser running, and use its profile as fallback. 3. If there no another browser running - load any existing profile. 4. If there no non-legacy-supervised profiles left - create a new one. 5. Mark existed/loaded/created profile as active and pass it to the callback. From user perspective logic become much more clear, if we take some action which kill current browser - we either switch to another open browser, or open a new one with appropriate profile. No more platform specific behaviour or special rules dependant on total profile count. What changed: * Removed Mac-specific case when it can open a new browser window while there is browser with another profile running. * On non-Mac platforms deleting active profile with no other browser running will not terminate browser process, instead a new browser window will be oppened with existed or newly created profile. BUG= R=anthonyvd@chromium.org, bauerb@chromium.org ==========
On 2016/11/22 15:10:52, Bernhard Bauer wrote: > On 2016/11/22 14:34:15, palar wrote: > > On 2016/11/22 14:16:45, Bernhard Bauer wrote: > > > This changes existing behavior, right? Specifically, if > > > > > > a) we're on Linux or Windows, > > > b) there is more than one profile, and > > > c) the currently active profile is deleted, > > > > > > we will now open a new window with the remaining profile, whereas we used to > > > quit, correct? Could you explain in the CL description why we want to do > that? > > > > > > > > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > > > File chrome/browser/profiles/profile_manager.cc (right): > > > > > > > > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > > > chrome/browser/profiles/profile_manager.cc:1326: for (auto* browser : > > > *BrowserList::GetInstance()) { > > > Use the real type instead of auto? > > > > > > > > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > > > chrome/browser/profiles/profile_manager.cc:1361: #if !defined(OS_CHROMEOS) > && > > > !defined(OS_ANDROID) > > > As an aside, this is already inside of an !defined(OS_ANDROID) block :) > > > > From user perspective logic become much more clear, if we take some > > action which kill current browser - we either switch to another open > > browser, or open a new one with appropriate profile. No more platform > > specific behaviour or special rules dependant on total profile count. > > OK, I think that makes sense. Can we make the change in behavior more explicit > in the CL though? Something like "Remove Mac special case when deleting the > active profile" as the first line. > > Also, +ewald@ (Identity PM) as FYI. What changed: * Removed Mac-specific case when it can open a new browser window while there is browser with another profile running. * On non-Mac platforms deleting active profile with no other browser running will not terminate browser process, instead a new browser window will be oppened with existed or newly created profile.
On 2016/11/22 15:50:01, palar wrote: > On 2016/11/22 15:10:52, Bernhard Bauer wrote: > > On 2016/11/22 14:34:15, palar wrote: > > > On 2016/11/22 14:16:45, Bernhard Bauer wrote: > > > > This changes existing behavior, right? Specifically, if > > > > > > > > a) we're on Linux or Windows, > > > > b) there is more than one profile, and > > > > c) the currently active profile is deleted, > > > > > > > > we will now open a new window with the remaining profile, whereas we used > to > > > > quit, correct? Could you explain in the CL description why we want to do > > that? > > > > > > > > > > > > > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > > > > File chrome/browser/profiles/profile_manager.cc (right): > > > > > > > > > > > > > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > > > > chrome/browser/profiles/profile_manager.cc:1326: for (auto* browser : > > > > *BrowserList::GetInstance()) { > > > > Use the real type instead of auto? > > > > > > > > > > > > > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > > > > chrome/browser/profiles/profile_manager.cc:1361: #if !defined(OS_CHROMEOS) > > && > > > > !defined(OS_ANDROID) > > > > As an aside, this is already inside of an !defined(OS_ANDROID) block :) > > > > > > From user perspective logic become much more clear, if we take some > > > action which kill current browser - we either switch to another open > > > browser, or open a new one with appropriate profile. No more platform > > > specific behaviour or special rules dependant on total profile count. > > > > OK, I think that makes sense. Can we make the change in behavior more explicit > > in the CL though? Something like "Remove Mac special case when deleting the > > active profile" as the first line. > > > > Also, +ewald@ (Identity PM) as FYI. > > What changed: > * Removed Mac-specific case when it can open a new browser window while > there is browser with another profile running. > * On non-Mac platforms deleting active profile with no other browser > running will not terminate browser process, instead a new browser > window will be oppened with existed or newly created profile. I'm not sure I understand the first bullet. On a high-level, it makes sense to me that: - We should have consistent behavior across all platforms - We should open a new browser window in the last used (or newly created) profile, if the active profile is deleted
On 2016/11/22 16:01:08, ewald wrote: > On 2016/11/22 15:50:01, palar wrote: > > On 2016/11/22 15:10:52, Bernhard Bauer wrote: > > > On 2016/11/22 14:34:15, palar wrote: > > > > On 2016/11/22 14:16:45, Bernhard Bauer wrote: > > > > > This changes existing behavior, right? Specifically, if > > > > > > > > > > a) we're on Linux or Windows, > > > > > b) there is more than one profile, and > > > > > c) the currently active profile is deleted, > > > > > > > > > > we will now open a new window with the remaining profile, whereas we > used > > to > > > > > quit, correct? Could you explain in the CL description why we want to do > > > that? > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > > > > > File chrome/browser/profiles/profile_manager.cc (right): > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > > > > > chrome/browser/profiles/profile_manager.cc:1326: for (auto* browser : > > > > > *BrowserList::GetInstance()) { > > > > > Use the real type instead of auto? > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > > > > > chrome/browser/profiles/profile_manager.cc:1361: #if > !defined(OS_CHROMEOS) > > > && > > > > > !defined(OS_ANDROID) > > > > > As an aside, this is already inside of an !defined(OS_ANDROID) block :) > > > > > > > > From user perspective logic become much more clear, if we take some > > > > action which kill current browser - we either switch to another open > > > > browser, or open a new one with appropriate profile. No more platform > > > > specific behaviour or special rules dependant on total profile count. > > > > > > OK, I think that makes sense. Can we make the change in behavior more > explicit > > > in the CL though? Something like "Remove Mac special case when deleting the > > > active profile" as the first line. > > > > > > Also, +ewald@ (Identity PM) as FYI. > > > > What changed: > > * Removed Mac-specific case when it can open a new browser window while > > there is browser with another profile running. > > * On non-Mac platforms deleting active profile with no other browser > > running will not terminate browser process, instead a new browser > > window will be oppened with existed or newly created profile. > > I'm not sure I understand the first bullet. On a high-level, it makes sense to > me that: > - We should have consistent behavior across all platforms > - We should open a new browser window in the last used (or newly created) > profile, if the active profile is deleted You're right. First bullet scenario: 0. Mac with sole profile (A) 1. Create profile B (browser window with profile B will open) 2. Create profile C (browser window with profile C will open) 3. Close browser (A). 4. From browser (C) delete profile C. Was: browser window with profile A will open Now: browser window with profile B become active.
On 2016/11/22 16:09:48, palar wrote: > On 2016/11/22 16:01:08, ewald wrote: > > On 2016/11/22 15:50:01, palar wrote: > > > On 2016/11/22 15:10:52, Bernhard Bauer wrote: > > > > On 2016/11/22 14:34:15, palar wrote: > > > > > On 2016/11/22 14:16:45, Bernhard Bauer wrote: > > > > > > This changes existing behavior, right? Specifically, if > > > > > > > > > > > > a) we're on Linux or Windows, > > > > > > b) there is more than one profile, and > > > > > > c) the currently active profile is deleted, > > > > > > > > > > > > we will now open a new window with the remaining profile, whereas we > > used > > > to > > > > > > quit, correct? Could you explain in the CL description why we want to > do > > > > that? > > > > > > > > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > > > > > > File chrome/browser/profiles/profile_manager.cc (right): > > > > > > > > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > > > > > > chrome/browser/profiles/profile_manager.cc:1326: for (auto* browser : > > > > > > *BrowserList::GetInstance()) { > > > > > > Use the real type instead of auto? > > > > > > > > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/2519953004/diff/1/chrome/browser/profiles/pro... > > > > > > chrome/browser/profiles/profile_manager.cc:1361: #if > > !defined(OS_CHROMEOS) > > > > && > > > > > > !defined(OS_ANDROID) > > > > > > As an aside, this is already inside of an !defined(OS_ANDROID) block > :) > > > > > > > > > > From user perspective logic become much more clear, if we take some > > > > > action which kill current browser - we either switch to another open > > > > > browser, or open a new one with appropriate profile. No more platform > > > > > specific behaviour or special rules dependant on total profile count. > > > > > > > > OK, I think that makes sense. Can we make the change in behavior more > > explicit > > > > in the CL though? Something like "Remove Mac special case when deleting > the > > > > active profile" as the first line. > > > > > > > > Also, +ewald@ (Identity PM) as FYI. > > > > > > What changed: > > > * Removed Mac-specific case when it can open a new browser window while > > > there is browser with another profile running. > > > * On non-Mac platforms deleting active profile with no other browser > > > running will not terminate browser process, instead a new browser > > > window will be oppened with existed or newly created profile. > > > > I'm not sure I understand the first bullet. On a high-level, it makes sense to > > me that: > > - We should have consistent behavior across all platforms > > - We should open a new browser window in the last used (or newly created) > > profile, if the active profile is deleted > > You're right. > > First bullet scenario: > 0. Mac with sole profile (A) > 1. Create profile B (browser window with profile B will open) > 2. Create profile C (browser window with profile C will open) > 3. Close browser (A). > 4. From browser (C) delete profile C. > Was: browser window with profile A will open > Now: browser window with profile B become active. ping
Can you update the CL title to something a bit more descriptive? Something like my suggestion in #msg6, or maybe a more general "Unify profile deletion logic across platforms".
Description was changed from ========== Profile deletion logic refactored. Profile deletion have quite fuzzy logic while deleting active profile: Sole profile deletion will open another window with newly created profile. If there is only one browser running, and there is another profile - on MacOS we will open browser with it, on other platforms browser will be closed. And finaly if there two browsers with different profiles running, and total three or more profiles - on MacOS new browser can or can not be opened. What we shall do: 1. If deletion happend with not active profile and not under guest profile - silently delete it. 2. If an active profile is pending for deletion - find another browser running, and use its profile as fallback. 3. If there no another browser running - load any existing profile. 4. If there no non-legacy-supervised profiles left - create a new one. 5. Mark existed/loaded/created profile as active and pass it to the callback. From user perspective logic become much more clear, if we take some action which kill current browser - we either switch to another open browser, or open a new one with appropriate profile. No more platform specific behaviour or special rules dependant on total profile count. What changed: * Removed Mac-specific case when it can open a new browser window while there is browser with another profile running. * On non-Mac platforms deleting active profile with no other browser running will not terminate browser process, instead a new browser window will be oppened with existed or newly created profile. BUG= R=anthonyvd@chromium.org, bauerb@chromium.org ========== to ========== Unify profile deletion logic across platforms. Profile deletion have quite fuzzy logic while deleting active profile: Sole profile deletion will open another window with newly created profile. If there is only one browser running, and there is another profile - on MacOS we will open browser with it, on other platforms browser will be closed. And finaly if there two browsers with different profiles running, and total three or more profiles - on MacOS new browser can or can not be opened. What we shall do: 1. If deletion happend with not active profile and not under guest profile - silently delete it. 2. If an active profile is pending for deletion - find another browser running, and use its profile as fallback. 3. If there no another browser running - load any existing profile. 4. If there no non-legacy-supervised profiles left - create a new one. 5. Mark existed/loaded/created profile as active and pass it to the callback. From user perspective logic become much more clear, if we take some action which kill current browser - we either switch to another open browser, or open a new one with appropriate profile. No more platform specific behaviour or special rules dependant on total profile count. What changed: * Removed Mac-specific case when it can open a new browser window while there is browser with another profile running. * On non-Mac platforms deleting active profile with no other browser running will not terminate browser process, instead a new browser window will be oppened with existed or newly created profile. BUG= R=anthonyvd@chromium.org, bauerb@chromium.org ==========
On 2016/11/23 16:14:41, Bernhard Bauer wrote: > Can you update the CL title to something a bit more descriptive? Something like > my suggestion in #msg6, or maybe a more general "Unify profile deletion logic > across platforms". done.
Thanks! LGTM.
The CQ bit was checked by palar@yandex-team.ru
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...)
lgtm % small nit https://codereview.chromium.org/2519953004/diff/20001/chrome/browser/profiles... File chrome/browser/profiles/profile_manager.cc (right): https://codereview.chromium.org/2519953004/diff/20001/chrome/browser/profiles... chrome/browser/profiles/profile_manager.cc:1325: // Serch for an active browser and use its profile as active if possible. nit: Search
On 2016/11/23 21:03:17, anthonyvd wrote: > lgtm % small nit > > https://codereview.chromium.org/2519953004/diff/20001/chrome/browser/profiles... > File chrome/browser/profiles/profile_manager.cc (right): > > https://codereview.chromium.org/2519953004/diff/20001/chrome/browser/profiles... > chrome/browser/profiles/profile_manager.cc:1325: // Serch for an active browser > and use its profile as active if possible. > nit: Search Typo fixed.
The CQ bit was checked by palar@yandex-team.ru
The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org, anthonyvd@chromium.org Link to the patchset: https://codereview.chromium.org/2519953004/#ps40001 (title: "Comment typo fixed.")
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: cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
The CQ bit was unchecked by palar@yandex-team.ru
The CQ bit was checked by palar@yandex-team.ru
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: cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by palar@yandex-team.ru
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: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by palar@yandex-team.ru
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": 40001, "attempt_start_ts": 1480062732426570,
"parent_rev": "f62502434012998cc68d3089e97a69606ee299c7", "commit_rev":
"a1756ed9132654bcc63de533e35b291a7f739bb3"}
Message was sent while issue was closed.
Description was changed from ========== Unify profile deletion logic across platforms. Profile deletion have quite fuzzy logic while deleting active profile: Sole profile deletion will open another window with newly created profile. If there is only one browser running, and there is another profile - on MacOS we will open browser with it, on other platforms browser will be closed. And finaly if there two browsers with different profiles running, and total three or more profiles - on MacOS new browser can or can not be opened. What we shall do: 1. If deletion happend with not active profile and not under guest profile - silently delete it. 2. If an active profile is pending for deletion - find another browser running, and use its profile as fallback. 3. If there no another browser running - load any existing profile. 4. If there no non-legacy-supervised profiles left - create a new one. 5. Mark existed/loaded/created profile as active and pass it to the callback. From user perspective logic become much more clear, if we take some action which kill current browser - we either switch to another open browser, or open a new one with appropriate profile. No more platform specific behaviour or special rules dependant on total profile count. What changed: * Removed Mac-specific case when it can open a new browser window while there is browser with another profile running. * On non-Mac platforms deleting active profile with no other browser running will not terminate browser process, instead a new browser window will be oppened with existed or newly created profile. BUG= R=anthonyvd@chromium.org, bauerb@chromium.org ========== to ========== Unify profile deletion logic across platforms. Profile deletion have quite fuzzy logic while deleting active profile: Sole profile deletion will open another window with newly created profile. If there is only one browser running, and there is another profile - on MacOS we will open browser with it, on other platforms browser will be closed. And finaly if there two browsers with different profiles running, and total three or more profiles - on MacOS new browser can or can not be opened. What we shall do: 1. If deletion happend with not active profile and not under guest profile - silently delete it. 2. If an active profile is pending for deletion - find another browser running, and use its profile as fallback. 3. If there no another browser running - load any existing profile. 4. If there no non-legacy-supervised profiles left - create a new one. 5. Mark existed/loaded/created profile as active and pass it to the callback. From user perspective logic become much more clear, if we take some action which kill current browser - we either switch to another open browser, or open a new one with appropriate profile. No more platform specific behaviour or special rules dependant on total profile count. What changed: * Removed Mac-specific case when it can open a new browser window while there is browser with another profile running. * On non-Mac platforms deleting active profile with no other browser running will not terminate browser process, instead a new browser window will be oppened with existed or newly created profile. BUG= R=anthonyvd@chromium.org, bauerb@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Unify profile deletion logic across platforms. Profile deletion have quite fuzzy logic while deleting active profile: Sole profile deletion will open another window with newly created profile. If there is only one browser running, and there is another profile - on MacOS we will open browser with it, on other platforms browser will be closed. And finaly if there two browsers with different profiles running, and total three or more profiles - on MacOS new browser can or can not be opened. What we shall do: 1. If deletion happend with not active profile and not under guest profile - silently delete it. 2. If an active profile is pending for deletion - find another browser running, and use its profile as fallback. 3. If there no another browser running - load any existing profile. 4. If there no non-legacy-supervised profiles left - create a new one. 5. Mark existed/loaded/created profile as active and pass it to the callback. From user perspective logic become much more clear, if we take some action which kill current browser - we either switch to another open browser, or open a new one with appropriate profile. No more platform specific behaviour or special rules dependant on total profile count. What changed: * Removed Mac-specific case when it can open a new browser window while there is browser with another profile running. * On non-Mac platforms deleting active profile with no other browser running will not terminate browser process, instead a new browser window will be oppened with existed or newly created profile. BUG= R=anthonyvd@chromium.org, bauerb@chromium.org ========== to ========== Unify profile deletion logic across platforms. Profile deletion have quite fuzzy logic while deleting active profile: Sole profile deletion will open another window with newly created profile. If there is only one browser running, and there is another profile - on MacOS we will open browser with it, on other platforms browser will be closed. And finaly if there two browsers with different profiles running, and total three or more profiles - on MacOS new browser can or can not be opened. What we shall do: 1. If deletion happend with not active profile and not under guest profile - silently delete it. 2. If an active profile is pending for deletion - find another browser running, and use its profile as fallback. 3. If there no another browser running - load any existing profile. 4. If there no non-legacy-supervised profiles left - create a new one. 5. Mark existed/loaded/created profile as active and pass it to the callback. From user perspective logic become much more clear, if we take some action which kill current browser - we either switch to another open browser, or open a new one with appropriate profile. No more platform specific behaviour or special rules dependant on total profile count. What changed: * Removed Mac-specific case when it can open a new browser window while there is browser with another profile running. * On non-Mac platforms deleting active profile with no other browser running will not terminate browser process, instead a new browser window will be oppened with existed or newly created profile. BUG= R=anthonyvd@chromium.org, bauerb@chromium.org Committed: https://crrev.com/198d711e1e047b20d235c430a697f1b39a651b08 Cr-Commit-Position: refs/heads/master@{#434463} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/198d711e1e047b20d235c430a697f1b39a651b08 Cr-Commit-Position: refs/heads/master@{#434463} |
