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

Issue 24227010: Revert 224514 "Fixed problems with pin/unpin preferences changin..." (Closed)

Created:
7 years, 3 months ago by piman
Modified:
7 years, 3 months ago
CC:
chromium-reviews
Visibility:
Public.

Description

Revert 224514 "Fixed problems with pin/unpin preferences changin..." Likely cause of http://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASAN%20Tests%20%283%29/builds/10105/steps/unit_tests/logs/stdio > Fixed problems with pin/unpin preferences changing for not running applications & icon order changes upon sync operations > > There are multiple problems fixed with this patch: > 1. When a running and pinned app (Windowed V1, V2) gets unpinned via sync operation (or multi user switching), the icon will disappear and the sytem gets into an unstable state (-> Crash later on). > 2. When a running app (Windowed V1, V2) gets pinned via a sync operation (or multi user switching), the icon will be shown, but get into an unsecure state which can lead to crashes. > 3. There are several cases, where the chrome browser icon does change locations due to sync / multi user switching operations. In this case the icon can shift by one spot to the right. > 4. There were some problems with the alternate shelf layout. > > That all said: With M32 we should possibly come up with a new way of storing running V1/V2 applications since the running applications are not stored by the preferences and can therefore flip around when switching between users AND windowed V1 apps currently have the same "weight" as an application launcher - which means that they will "float around" in the shelf between sync / switch operations. > > My visual test to see that everything was working was as follows: > > In Multi session mode, have one user with the following applications pinned: > Chrome Shop, VNC, Angry Birds, gmail, calendar, chrome > and another user with two other applications and chrome. > > Switch between the two and everything should be fine. > Run VNC (V2 app). Switch to user 2 and see that it is unpinned at the end of the list. Switch back and see it sliding back to where it belongs. > Then do the same for a windowed V1 app (gmail). Note that the gmail item will not float to the end of the list in user 2 - which is because of the already mentioned weights. > BUG=294435, 294231, 294227 > TEST=unit tests and visual (see above) > R=oshima@chromium.org > > Review URL: https://codereview.chromium.org/23580008 TBR=skuhne@google.com Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=224529

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+94 lines, -561 lines) Patch
M trunk/src/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h View 4 chunks +18 lines, -28 lines 0 comments Download
M trunk/src/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc View 9 chunks +73 lines, -176 lines 0 comments Download
M trunk/src/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc View 10 chunks +3 lines, -357 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
piman
7 years, 3 months ago (2013-09-21 00:08:40 UTC) #1
piman
Committed patchset #1 manually as r224529.
7 years, 3 months ago (2013-09-21 00:08:51 UTC) #2
Mr4D (OOO till 08-26)
7 years, 3 months ago (2013-09-21 01:36:31 UTC) #3
Message was sent while issue was closed.
lgtm

Powered by Google App Engine
This is Rietveld 408576698