GetAppContainerSidForSandboxType needs work for side-by-side dev and beta |
|
Issue description
While browsing code, I noticed that ChromeContentBrowserClient::GetAppContainerSidForSandboxType appears to be lacking independent SIDs for dev and beta.
A better model than having #ifdef GOOGLE_CHROME_BUILD and switching on channels is to add a sid member to install_static::InstallConstants and then add values to the install_static::kInstallModes definitions for Chromium (chrome/install_static/chromium_install_modes.cc) and Google Chrome (chrome/install_static/google_chrome_install_modes.cc).
GetAppContainerSidForSandboxType can then do something like:
sid.assign(install_static::GetSandboxSid());
where GetSandboxSid is:
const wchar_t* GetSandboxSid() { return InstallDetails::Get().sandbox_sid; }
in install_static/install_util.{cc,h}
Do you have cycles for this, Will?
,
May 31 2018
I wonder if it's worth maintaining different code for all this, especially as the GetAppContainerSidForSandboxType function is only required to work around a layering violation. For full GPU AC work what I actually ended up doing was generating the SID based on the main executable's path, in part because it turned out not worth the effort to add a new function to a browser client class. As we don't really care about the absolute value of the SID but rather it's uniqueness WRT to other AC's perhaps we can do the same thing here and remove the entire code. That way we'd get new AC SIDs for free.
,
Jun 6 2018
I would like to take this up!
,
Jun 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a0de7ce9be3e4d912e74ccbe44e622ba5d12924e commit a0de7ce9be3e4d912e74ccbe44e622ba5d12924e Author: Jerry Lin <wahahab11@gmail.com> Date: Fri Jun 22 15:18:36 2018 Make GetAppContainerSidForSandboxType get sid from install details. This CL remove hard code SID in function |ChromeContentBrowserClient::GetAppContainerSidForSandboxType|. The SIDs are moved to structure |InstallConstants| and helper function |GetSandboxSid| are added for getting SID from that structure. Bug: 845876 Change-Id: I553663ffd6613fe61ebe6c3a36c233dce42d3fc5 Reviewed-on: https://chromium-review.googlesource.com/1094576 Commit-Queue: Greg Thompson <grt@chromium.org> Reviewed-by: Greg Thompson <grt@chromium.org> Cr-Commit-Position: refs/heads/master@{#569633} [modify] https://crrev.com/a0de7ce9be3e4d912e74ccbe44e622ba5d12924e/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/a0de7ce9be3e4d912e74ccbe44e622ba5d12924e/chrome/install_static/chromium_install_modes.cc [modify] https://crrev.com/a0de7ce9be3e4d912e74ccbe44e622ba5d12924e/chrome/install_static/google_chrome_install_modes.cc [modify] https://crrev.com/a0de7ce9be3e4d912e74ccbe44e622ba5d12924e/chrome/install_static/install_constants.h [modify] https://crrev.com/a0de7ce9be3e4d912e74ccbe44e622ba5d12924e/chrome/install_static/install_util.cc [modify] https://crrev.com/a0de7ce9be3e4d912e74ccbe44e622ba5d12924e/chrome/install_static/install_util.h [modify] https://crrev.com/a0de7ce9be3e4d912e74ccbe44e622ba5d12924e/chrome/install_static/install_util_unittest.cc |
|
►
Sign in to add a comment |
|
Comment 1 by wfh@chromium.org
, May 30 2018Labels: Hotlist-GoodFirstBug
Owner: ----
Status: Available (was: Assigned)