New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 726016 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug-Regression



Sign in to add a comment

local builds and system Chrome use same profile in CRD

Project Member Reported by skobes@chromium.org, May 24 2017

Issue description

Normally I can run local Chromium builds side-by-side with the system Chrome installation on my Linux workstation, because they use different profile paths:

  ~/.config/google-chrome/Default  # system Chrome
  ~/.config/chromium/Default  # local Chromium build

But inside a Chrome Remote Desktop session, both of them try to use this path:

  ~/.config/chrome-remote-desktop/chrome-profile/Default

This causes a lot of errors like:

  "Your profile can not be used because it is from a newer version of Google Chrome."

As a workaround, I can pass an explicit profile path with --user-data-dir.

It would be nice if these could run side-by-side in CRD by default. 
 For example, if local Chromium builds used ~/.config/chromium/Default regardless of whether they are inside CRD.

I think this used to work in the past, so I'm tagging it as a regression.  It might be a fairly old regression though.
 

Comment 1 by skobes@chromium.org, May 24 2017

Labels: -Type-Bug Type-Bug-Regression
CRD sets an environment variable $CHROME_USER_DATA_DIR in the virtual session it creates.
That is probably why your system Chrome and your local Chromium are using the same profile path.

I don't know about any regression, but there was a time (years ago) when CRD did not set this variable. In that case, system-Chrome launched from CRD would share its profile with system-Chrome launched from the physical desktop. That caused Chrome windows to appear on the wrong desktop if Chrome was already running.

Comment 3 by skobes@chromium.org, May 24 2017

Maybe Chromium should ignore $CHROME_USER_DATA_DIR unless is_official_build is set.
On Linux, it's possible to run several different versions side-by-side, which is achieved by having each one use a different profile directory. I think they will all be broken under CRD because the launch scripts don't do any profile redirection if CHROME_USER_DATA_DIR is set (see https://codereview.chromium.org/190473005).

Short of fixing Chrome's one-DISPLAY-per-profile restriction, the only solution I can see is to have a separate env variable for each version of Chrome that can be installed side-by-side, but it's an ugly fix for a problem that I suspect affects a very small set of users. Just ignoring the variable for non-official builds will solve your use-case but break any Chromium users who want to run concurrently on the console and in a CRD session, so I'm not convinced it's a net win.

Comment 5 by skobes@chromium.org, May 24 2017

The GetDefaultUserDataDirectory function in chrome/common/chrome_paths_linux.cc has the following code:

#if defined(GOOGLE_CHROME_BUILD)
  *result = config_dir.Append("google-chrome");
#else
  *result = config_dir.Append("chromium");
#endif

Perhaps this logic could also be applied to the codepath that handles the $CHROME_USER_DATA_DIR override.  In other words the env var is treated as a prefix, and the real profile path in CRD becomes

  ~/.config/chrome-remote-desktop/chrome-profile/{google-chrome,chromium}
Status: Untriaged (was: Unconfirmed)
I don't think we can just change the semantics of the existing variable because CRD users would lose their profile data when Chrome started using ~/.config/chrome-remote-desktop/chrome-profile/chrome. A different variable, used in preference to CHROME_USER_DATA_DIR if both are set, to which either /chrome-profile or /chromium-profile is appended would solve that problem and allow existing users to retain their profile data.

I still think this introduces complexity for not a lot of user benefit, but as long as there's no regression for CRD users (and anyone else who might be relying on CHROME_USER_DATA_DIR), I have no strong objection.

Components: -Services>Chromoting
Owner: skobes@chromium.org
Status: Assigned (was: Untriaged)
From a CRD perspective, we don't have a strong opinion on this. Please loop me in on any CL if you can convince the relevant OWNERS that it's worth fixing.

Comment 8 by skobes@chromium.org, May 25 2017

Hmm, a new env variable won't actually solve the problem mentioned in #6, since existing users will get the new path as soon as CRD starts setting it in the virtual session.

It sounds like you're saying the default profile path is essentially set in stone for eternity?  Unless we implement some kind of auto-migration scheme, which seems more trouble than it's worth.

Was there any concern about losing profile data when $CHROME_USER_DATA_DIR was first introduced?

Comment 9 by skobes@chromium.org, Jun 21 2017

Status: Started (was: Assigned)
Update: my initial proposal in http://crrev.com/c/516729 won't work as-is because the wrapper script passes --user-data-dir=$CHROME_USER_DATA_DIR:

  https://cs.chromium.org/chromium/src/chrome/installer/linux/common/wrapper?q=CHROME_USER_DATA_DIR

It seems this was added to override the channel-specific path which is *also* passed through --user-data-dir:

  https://cs.chromium.org/chromium/src/chrome/installer/linux/debian/build.sh?q=SXS_USER_DATA_DIR

The --user-data-dir flag takes precedence over anything in the environment, so any change in default user data dir logic needs to be implemented redundantly in the Chrome binary and the wrapper script, and kept in sync.

New plan:

(step 1)  Stop passing --user-data-dir in the wrapper script.  This requires the default user data dir to include the channel suffix.  Channel is already passed in as $CHROME_VERSION_EXTRA and exposed (with some transformations) by chrome::GetChannelString, so this is a simple change.

I think it's a good idea regardless of fixing this bug, since it consolidates the user data dir logic in one place, making it easier to understand and modify.

(step 2)  Introduce $CHROME_CONFIG_HOME, used in preference to ~/.config by chrome::GetDefaultUserDataDirectory.  This is

- less magical than expanding placeholders in $CHROME_USER_DATA_DIR

- works alongside existing logic for channel- and product-specific (Chrome vs. Chromium) user data dirs

- similar to already-supported $XDG_CONFIG_HOME, except it won't impact other Linux applications

The user can then create ~/.chrome-remote-desktop-session like this:

#!/bin/sh
export CHROME_CONFIG_HOME="$HOME/.config/chrome-remote-desktop/chrome-config"
unset CHROME_USER_DATA_DIR
. /etc/chrome-remote-desktop-session
Patches up:
[1] http://crrev.com/c/545092 Include channel suffix in default user data directory on Linux.
[2] http://crrev.com/c/544804 Don't pass --user-data-dir in Linux launch wrapper script.
[3] http://crrev.com/c/544531 Support $CHROME_CONFIG_HOME in Linux default user data dir selection.
Cc: thomasanderson@chromium.org phajdan.jr@chromium.org
I think we had migration code when we first allowed SxD installs on Linux. Probably something Pawel wrote and I reviewed. You can look into the commit history for that.
Going to try to summarize this...

So the order for determining what user data dir to use is:

(A) --user-data-dir
(B) $CHROME_USER_DATA_DIR
(D) $XDG_CONFIG_HOME + branding

with $CHROME_VERSION_EXTRA as a Chrome-only modifier for (D).

And we are proposing to:
[1] Make it unnecessary to use (A) in the Chrome beta/dev channels' wrapper script, by deriving the channel-specific user data dir via $CHROME_VERSION_EXTRA in C++ instead.
[2] Stop using (A) in the wrapper scripts.
[3] Change the order to:

(A) --user-data-dir
(B) $CHROME_USER_DATA_DIR
(C) $CHROME_CONFIG_HOME + branding
(D) $XDG_CONFIG_HOME + branding

with $CHROME_VERSION_EXTRA as a Chrome-only modifier for (C) and (D).

The end result being:

CRD users are still stuck with (B) and we don't want to unset $CHROME_USER_DATA_DIR because that will disrupt existing users.

The suggested workaround for users that want to run Chrome (SxS) and Chromium in CRD is for them take a manual step to drop (B), and switch to (C). In which case, the users that switch to (C) will also have to manually migrate their CRD user data dir. And we still want to do all this because it will help new users, and it is less pain than making users use (A) manually.

Summary is correct.  I will just add that thanks to Chrome Sync I don't feel any need to manually migrate the user data dir when switching from (B) to (C) - I just sign in to the fresh profile and everything I care about is there.

Using (A) manually isn't really an option when the command line is constructed by some other program.  For example, the Blink layout test runner launches the system browser to display the test results.
Well, launching the system browser probably ends up reading /usr/share/applications/google-chrome.desktop, assuming that's the default, and looking for the Exec= line. Though having to override that every time Chrome updates would be painful.

The proposal sounds like a better user experience for those affected, at the cost of yet another environment variable. I'll start reviewing the CLs since nobody has objected.
Project Member

Comment 15 by bugdroid1@chromium.org, Jun 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2f7ff6f11ae536cc187f246e147fb03c21304713

commit 2f7ff6f11ae536cc187f246e147fb03c21304713
Author: Steve Kobes <skobes@chromium.org>
Date: Tue Jun 27 03:37:44 2017

Include channel suffix in default user data directory on Linux.

This matches what the launch wrapper script currently passes in --user-data-dir
for non-stable channels.  After this change lands, the wrapper script can stop
passing --user-data-dir.

Bug:  726016 
Change-Id: I2083fa9be16503ad0fbf498daa5f02de3301d708
Reviewed-on: https://chromium-review.googlesource.com/545092
Commit-Queue: Steve Kobes <skobes@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482531}
[modify] https://crrev.com/2f7ff6f11ae536cc187f246e147fb03c21304713/chrome/common/BUILD.gn
[modify] https://crrev.com/2f7ff6f11ae536cc187f246e147fb03c21304713/chrome/common/channel_info.h
[modify] https://crrev.com/2f7ff6f11ae536cc187f246e147fb03c21304713/chrome/common/channel_info_posix.cc
[modify] https://crrev.com/2f7ff6f11ae536cc187f246e147fb03c21304713/chrome/common/chrome_paths_linux.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Jun 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f0da5d0c526cddcb7b4d6fdf11fa4ec54ba84c9

commit 9f0da5d0c526cddcb7b4d6fdf11fa4ec54ba84c9
Author: Steve Kobes <skobes@chromium.org>
Date: Tue Jun 27 04:21:06 2017

Revert "Include channel suffix in default user data directory on Linux."

This reverts commit 2f7ff6f11ae5 (http://crrev.com/482531).

Reason for revert: Broke ChromeOS compile:
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20ChromeOS/builds/35706

TBR=jamiewalch@chromium.org,thestig@chromium.org,skobes@chromium.org

Change-Id: Ibd825677ec17f81b9a108d260480bd7295b4937d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  726016 
Reviewed-on: https://chromium-review.googlesource.com/549585
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482535}
[modify] https://crrev.com/9f0da5d0c526cddcb7b4d6fdf11fa4ec54ba84c9/chrome/common/BUILD.gn
[modify] https://crrev.com/9f0da5d0c526cddcb7b4d6fdf11fa4ec54ba84c9/chrome/common/channel_info.h
[modify] https://crrev.com/9f0da5d0c526cddcb7b4d6fdf11fa4ec54ba84c9/chrome/common/channel_info_posix.cc
[modify] https://crrev.com/9f0da5d0c526cddcb7b4d6fdf11fa4ec54ba84c9/chrome/common/chrome_paths_linux.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Jun 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d90e3bcca9f75282bbf0c17239c022e3124c654e

commit d90e3bcca9f75282bbf0c17239c022e3124c654e
Author: Steve Kobes <skobes@chromium.org>
Date: Tue Jun 27 07:25:10 2017

[Reland] Include channel suffix in default user data directory on Linux.

This matches what the launch wrapper script currently passes in --user-data-dir
for non-stable channels.  After this change lands, the wrapper script can stop
passing --user-data-dir.

This is a reland of http://crrev.com/c/545092 (r482531), reverted at r482535,
with a fix for the ChromeOS build.

TBR=jamiewalch@chromium.org,thestig@chromium.org,skobes@chromium.org

Bug:  726016 
Change-Id: I9499b618b1341c1df36e3b149e4f2458f3b34777
Reviewed-on: https://chromium-review.googlesource.com/549586
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482561}
[modify] https://crrev.com/d90e3bcca9f75282bbf0c17239c022e3124c654e/chrome/common/BUILD.gn
[modify] https://crrev.com/d90e3bcca9f75282bbf0c17239c022e3124c654e/chrome/common/channel_info.h
[modify] https://crrev.com/d90e3bcca9f75282bbf0c17239c022e3124c654e/chrome/common/channel_info_chromeos.cc
[modify] https://crrev.com/d90e3bcca9f75282bbf0c17239c022e3124c654e/chrome/common/channel_info_posix.cc
[modify] https://crrev.com/d90e3bcca9f75282bbf0c17239c022e3124c654e/chrome/common/chrome_paths_linux.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Jun 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0f113e930601811421c33eb361a431e595ec7eb9

commit 0f113e930601811421c33eb361a431e595ec7eb9
Author: Steve Kobes <skobes@chromium.org>
Date: Tue Jun 27 18:56:49 2017

Don't pass --user-data-dir in Linux launch wrapper script.

The logic for channel-specific user data dirs and the $CHROME_USER_DATA_DIR
override is now baked into the Chrome binary.

Bug:  726016 
Change-Id: Ib815497c07d4ad4c017a42abd43f1987d91149a2
Reviewed-on: https://chromium-review.googlesource.com/544804
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482699}
[modify] https://crrev.com/0f113e930601811421c33eb361a431e595ec7eb9/chrome/installer/linux/common/installer.include
[modify] https://crrev.com/0f113e930601811421c33eb361a431e595ec7eb9/chrome/installer/linux/common/wrapper
[modify] https://crrev.com/0f113e930601811421c33eb361a431e595ec7eb9/chrome/installer/linux/debian/build.sh
[modify] https://crrev.com/0f113e930601811421c33eb361a431e595ec7eb9/chrome/installer/linux/rpm/build.sh

Project Member

Comment 19 by bugdroid1@chromium.org, Jun 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c9ae6a1a1214f69cb9ff0180468c669e82a380e6

commit c9ae6a1a1214f69cb9ff0180468c669e82a380e6
Author: Steve Kobes <skobes@chromium.org>
Date: Tue Jun 27 21:39:02 2017

Support $CHROME_CONFIG_HOME in Linux default user data dir selection.

If set, it is used in place of ~/.config when constructing the path.  Users can
use this instead of $CHROME_USER_DATA_DIR to get a custom location that still
allows side-by-side profiles for different channels and branding.

Bug:  726016 
Change-Id: I2066fb4ea9bc409976042deaf4ee6689c714373c
Reviewed-on: https://chromium-review.googlesource.com/544531
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482753}
[modify] https://crrev.com/c9ae6a1a1214f69cb9ff0180468c669e82a380e6/chrome/common/chrome_paths_linux.cc
[modify] https://crrev.com/c9ae6a1a1214f69cb9ff0180468c669e82a380e6/chrome/common/chrome_paths_unittest.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Jun 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0e050276be9274b312a0768be1f10511c7b0d870

commit 0e050276be9274b312a0768be1f10511c7b0d870
Author: Steve Kobes <skobes@chromium.org>
Date: Fri Jun 30 21:20:11 2017

Make ChromePaths.DefaultUserDataDir pass with is_chrome_branded = true.

This test was added in r482753 and previously assumed a non-branded build.

Bug: 738511,  726016 
Change-Id: I8f7d98381c27e3d07a1a6bf8656a0db6e9c0f202
Reviewed-on: https://chromium-review.googlesource.com/557950
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483819}
[modify] https://crrev.com/0e050276be9274b312a0768be1f10511c7b0d870/chrome/common/chrome_paths_unittest.cc

Project Member

Comment 21 by bugdroid1@chromium.org, Jul 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/22fb19cd6e7b239131159b9ef118da0fe427511d

commit 22fb19cd6e7b239131159b9ef118da0fe427511d
Author: Steve Kobes <skobes@chromium.org>
Date: Wed Jul 05 21:49:08 2017

Document user data directory logic.

Incorporates all of https://www.chromium.org/user-experience/user-data-directory
which can now be a redirect.  Adds some things that weren't documented,
including guidance on the new $CHROME_CONFIG_HOME variable.

Bug:  726016 
Change-Id: I91944f04ffbcfbd268c8d18d2121ffd1b5f54a70
Reviewed-on: https://chromium-review.googlesource.com/558653
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484358}
[modify] https://crrev.com/22fb19cd6e7b239131159b9ef118da0fe427511d/docs/README.md
[add] https://crrev.com/22fb19cd6e7b239131159b9ef118da0fe427511d/docs/user_data_dir.md

Project Member

Comment 22 by bugdroid1@chromium.org, Jul 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e26f53bdb423a8c9663ec9d28faa53ad6edd77f0

commit e26f53bdb423a8c9663ec9d28faa53ad6edd77f0
Author: Steve Kobes <skobes@chromium.org>
Date: Thu Jul 06 00:15:02 2017

Fix broken links in user_data_dir.md.

Gitiles is stricter than md_browser about URL-encoding parentheses.

Bug:  726016 
Change-Id: Ifc741701afd65300b0ad057316b186b4126f5aa7
Reviewed-on: https://chromium-review.googlesource.com/560559
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484390}
[modify] https://crrev.com/e26f53bdb423a8c9663ec9d28faa53ad6edd77f0/docs/user_data_dir.md

Project Member

Comment 23 by bugdroid1@chromium.org, Jul 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4966641a02c3fdc4d51527cad5c7f5673ea3176b

commit 4966641a02c3fdc4d51527cad5c7f5673ea3176b
Author: Steve Kobes <skobes@chromium.org>
Date: Thu Jul 06 01:02:49 2017

Fix broken link in user_data_dir.md, one more try.

Bug:  726016 
Change-Id: I4c7f0e33e9ef305468934688c76ded5d6942aa54
Reviewed-on: https://chromium-review.googlesource.com/560623
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484430}
[modify] https://crrev.com/4966641a02c3fdc4d51527cad5c7f5673ea3176b/docs/user_data_dir.md

Status: Fixed (was: Started)
http://i.imgur.com/kdsEi3E.gifv
 Issue 752678  has been merged into this issue.

Sign in to add a comment