|
|
Created:
6 years, 10 months ago by Matt Giuca Modified:
6 years, 1 month ago Reviewers:
jackhou1, Lei Zhang, Will Harris, oshima, robertshield, security, James Hawkins, grt (UTC plus 2) CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org, tfarina, chromium-apps-reviews_chromium.org, robertshield Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionAdding app_shim executable on Windows.
This is a small binary that runs Chrome with the given command-line
arguments. It is intended for use with Chrome apps, so that file types
can be associated with the shim, having a custom name (and in some
cases, icon) rather than simply the name of the Chrome binary.
BUG=130455
Committed: https://crrev.com/31d809c90a47afe763ea6a027d811eae1707914f
Cr-Commit-Position: refs/heads/master@{#302749}
Patch Set 1 #Patch Set 2 : Rebase. #Patch Set 3 : Added icon for win_app_shim (currently just the webstore ico). #Patch Set 4 : Rebase (the shim directory was moved). #Patch Set 5 : Move webstore.ico to a more sensible location. #Patch Set 6 : Cleanup and copyright notice. #
Total comments: 5
Patch Set 7 : Moved webstore.ico into win subdirectory. #
Total comments: 15
Patch Set 8 : Rebase. #Patch Set 9 : Renamed win_app_shim.exe to app_shim_win.exe. #Patch Set 10 : Automatically get the Chrome binary from the registry instead of taking it on the command line. #
Total comments: 15
Patch Set 11 : Added manifest, moved and renamed files and targets. #Patch Set 12 : Respond to grt review. #
Total comments: 2
Patch Set 13 : Alphabetical order. #
Messages
Total messages: 39 (15 generated)
Patchset #4 (id:80001) has been deleted
mgiuca@chromium.org changed reviewers: + jackhou@chromium.org, oshima@chromium.org
Rationale is in the comment at the top of app_shim_win.cc. jackhou: chrome/app_shim/* oshima: chrome/app/theme/.../webstore.ico Note that this icon is temporary; I will get a UI designer to make a proper icon for this executable. For now, it is just a simple icon to represent "a generic Chrome app". Some notes are inline. https://codereview.chromium.org/179223003/diff/130001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.cc (right): https://codereview.chromium.org/179223003/diff/130001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:49: // somehow, instead of taking it as a command-line argument. I've left this TODO in so maybe we can have a discussion about it. Someone (I've forgotten who) suggested that I do this. I see there is code to do this in chrome/installer/launcher_support/chrome_launcher_support.h, but I'm not sure if it will work in this case --- it needs to open the exact same Chrome as that which created it. (For example, Chrome Canary or Chrome Stable.) Will this work? It also makes it hard to test, since regular Chromium won't be installed in the registry in this way. https://codereview.chromium.org/179223003/diff/130001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.gypi (right): https://codereview.chromium.org/179223003/diff/130001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.gypi:6: 'targets': [ Note: I tried putting this in app_shim.gypi, but GYP complained that there was a variable that was undefined. Because I think it's only available on Mac. We could break up app_shim.gypi into Mac-only and Windows-only sections, but I figured it was simpler to just create a new file. Is that OK? (If so, we should rename app_shim.gypi to app_shim_mac.gypi.)
c/a/t lgtm
https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.cc (right): https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:23: // This program takes a full Windows command line as command-line arguments, and I remember discussing this a while back and saying that it should be fine to just pass all the arguments. But if the set of switches we expect to pass with this is small, maybe we should just parse them. That greatly limits what this binary is capable of. https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:41: logging_settings.logging_dest = logging::LOG_TO_SYSTEM_DEBUG_LOG; I was under the impression stderr doesn't work on Windows and LOG_TO_SYSTEM_DEBUG_LOG uses Sawbuck. But you're able to log to stderr with this, then ignore this comment. If you want to use Sawbuck, the instructions are here: https://code.google.com/p/sawbuck/ You can take a look at how I've done it here: https://codereview.chromium.org/423293004/diff/410001/chrome/app_installer/wi... https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:48: // TODO(mgiuca): Consider implicitly finding chrome.exe through the registry Code to do this is here: https://code.google.com/p/chromium/codesearch#chromium/src/chrome/installer/l... I'm adding GetAnyChromeSxSPath() in https://codereview.chromium.org/423293004/ which should land "soon". https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.gypi (right): https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.gypi:22: 'msvs_settings': { The other Windows exe's have a manifest file. I'm not sure if it's essential though. https://code.google.com/p/chromium/codesearch#search/&q=exe.manifest&sq=packa...
Oops, sorry, I missed your earlier comments. https://codereview.chromium.org/179223003/diff/130001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.cc (right): https://codereview.chromium.org/179223003/diff/130001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:49: // somehow, instead of taking it as a command-line argument. On 2014/10/24 09:45:46, Matt Giuca wrote: > I've left this TODO in so maybe we can have a discussion about it. Someone (I've > forgotten who) suggested that I do this. > > I see there is code to do this in > chrome/installer/launcher_support/chrome_launcher_support.h, but I'm not sure if > it will work in this case --- it needs to open the exact same Chrome as that > which created it. (For example, Chrome Canary or Chrome Stable.) Will this work? > > It also makes it hard to test, since regular Chromium won't be installed in the > registry in this way. Yeah that's a good point. The only problem I can think of is if Chrome is installed at a user level, then later at a system level. The system level install will replace the user level one, so hard coded chrome.exe paths will break. If there's no security issue with passing the path, we could do both, and only use the hard coded path for Chromium. https://codereview.chromium.org/179223003/diff/130001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.gypi (right): https://codereview.chromium.org/179223003/diff/130001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.gypi:6: 'targets': [ On 2014/10/24 09:45:46, Matt Giuca wrote: > Note: I tried putting this in app_shim.gypi, but GYP complained that there was a > variable that was undefined. Because I think it's only available on Mac. > > We could break up app_shim.gypi into Mac-only and Windows-only sections, but I > figured it was simpler to just create a new file. Is that OK? (If so, we should > rename app_shim.gypi to app_shim_mac.gypi.) Yeah that's fine. I'll rename the mac one.
mgiuca@chromium.org changed reviewers: + grt@chromium.org, security@chromium.org
+security: Can someone from security team look at this? In the comments attached to this email, I have added "SECURITY" on questions I have for the security team. Also: Is it necessary to have a security review before committing the code (or just to launch)? Note that as of this CL, the binary won't be packaged with Chrome so there is no danger of this feature "shipping" without approval. +grt: Jack suggested you look at this in case you have any tips for adding a new binary. (Note: It is not my intention to package the shim with the release for this particular CL.) https://codereview.chromium.org/179223003/diff/130001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.cc (right): https://codereview.chromium.org/179223003/diff/130001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:49: // somehow, instead of taking it as a command-line argument. SECURITY: Can you comment on whether it is safe to have the shim binary able to open any arbitrary executable on the system (as opposed to only being able to open Chrome). The way I see it, an attacker can use this binary to run an arbitrary command line. But this is only possible if the attacker is able to run the shim with an arbitrary command line --- so it doesn't give you any new privileges you didn't already have. The only concern we have is that it would mean a Google-signed binary is potentially able to execute any other binary on the system. Does that have any negative implications? https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.cc (right): https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:23: // This program takes a full Windows command line as command-line arguments, and SECURITY: Do you think it is necessary to restrict the set of command-line switches which the shim can pass to Chrome? (This makes our life potentially harder because if we ever want to pass a new argument to Chrome from the shim, we have to update the shim.) https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:41: logging_settings.logging_dest = logging::LOG_TO_SYSTEM_DEBUG_LOG; This is really only used to log the usage message below (which is only useful if someone is trying to manually run the shim to understand how it works). It does log to the command line output. (I could just use printf... but this way seems neater.) https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:48: // TODO(mgiuca): Consider implicitly finding chrome.exe through the registry On 2014/10/26 22:56:09, jackhou1 wrote: > Code to do this is here: > > https://code.google.com/p/chromium/codesearch#chromium/src/chrome/installer/l... > > I'm adding GetAnyChromeSxSPath() in https://codereview.chromium.org/423293004/ > which should land "soon". Acknowledged.
wfh@chromium.org changed reviewers: + wfh@chromium.org
https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.cc (right): https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:23: // This program takes a full Windows command line as command-line arguments, and On 2014/10/27 00:16:16, Matt Giuca wrote: > SECURITY: Do you think it is necessary to restrict the set of command-line > switches which the shim can pass to Chrome? > > (This makes our life potentially harder because if we ever want to pass a new > argument to Chrome from the shim, we have to update the shim.) +robertshield The concern I have is that malware might piggyback on Chrome's digital signature to load it's binaries via this signed executable, since AV typically uses whether an executable is signed as a heuristic to determine how much to trust it. Robert, what do you think?
robertshield@chromium.org changed reviewers: + robertshield@chromium.org
https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.cc (right): https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:23: // This program takes a full Windows command line as command-line arguments, and On 2014/10/27 16:09:31, Will Harris wrote: > On 2014/10/27 00:16:16, Matt Giuca wrote: > > SECURITY: Do you think it is necessary to restrict the set of command-line > > switches which the shim can pass to Chrome? > > > > (This makes our life potentially harder because if we ever want to pass a new > > argument to Chrome from the shim, we have to update the shim.) > > +robertshield > > The concern I have is that malware might piggyback on Chrome's digital signature > to load it's binaries via this signed executable, since AV typically uses > whether an executable is signed as a heuristic to determine how much to trust > it. Robert, what do you think? I'd be much less worried if PATH_TO_CHROME_EXE stops being an argument and is instead figured out by other means. There's a TODO below to do just this that mentions https://codereview.chromium.org/423293004/, landing that first sounds like a good idea. With the above change, I don't immediately see how a malware author would benefit from running this vs. running Chrome directly. Presumably if an attacker can already execute an arbitrary command line then trouble is already afoot. https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.gypi (right): https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.gypi:22: 'msvs_settings': { On 2014/10/26 22:56:09, jackhou1 wrote: > The other Windows exe's have a manifest file. I'm not sure if it's essential > though. > https://code.google.com/p/chromium/codesearch#search/&q=exe.manifest&sq=packa... You want one to avoid a bunch of shimming that happens if don't have one. This app is pretty simple today so it might not matter, but to avoid future headaches, strongly recommend adding one.
Patchset #10 (id:200001) has been deleted
PTAL. https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.cc (right): https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:23: // This program takes a full Windows command line as command-line arguments, and OK I'm using the registry to get the Chrome binary path. We still allow arbitrary command-line flags. This shouldn't be a problem on Chromium because I've got another patch to get the Chromium binary path from the registry. (It still needs to be installed, but it does work for Chromium now.) https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.gypi (right): https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.gypi:22: 'msvs_settings': { OK, I haven't really looked into this. Since it doesn't seem to be necessary right now, I think I will go ahead without one and look into adding one alongside the CL where we bundle the exe with Chrome. What is "shimming" in this context?
LGTM You'll need an owner for chrome.gyp
https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.cc (right): https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:13: #include <windows.h> move above line 5 as per http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Names_and_Ord.... https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:46: int WINAPI WinMain(HINSTANCE instance, WinMain -> wWinMain https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:75: if (!base::LaunchProcess(command_line, options, nullptr)) options -> base::LaunchOptions() https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:76: return kLaunchFailure; LOG(ERROR) here as on line 63? https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.gypi (right): https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.gypi:10: 'target_name': 'app_shim_win', it seems that this does something completely different than the "app_shim" for OSX in this directory. overloading the term like this seems like a mistake. is there a more appropriate name/location for this component? https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.gypi:13: '../..', is this necessary?
I'm not at a computer until Monday so I will reply to the rest of the comments then. https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.gypi (right): https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.gypi:10: 'target_name': 'app_shim_win', On 2014/10/30 17:04:56, grt wrote: > it seems that this does something completely different than the "app_shim" for > OSX in this directory. overloading the term like this seems like a mistake. is > there a more appropriate name/location for this component? It's the same concept: a small shim to run an app because the OS won't let us run Chrome directly, for whatever reason. The differences are: - The Mac one needs to stay resident for the lifetime of the app, whereas the Windows one can quit once the app is launched. - The Mac one is used to give the app a dock icon and it's own presence in the OS menu, as well as to provide a target for file associations. The Windows one is just to provide a target for for associations. Those differences aren't enough to justify using a different name. They do almost exactly the same thing and are both used for the same purpose.
https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.gypi (right): https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.gypi:10: 'target_name': 'app_shim_win', On 2014/10/30 21:16:35, Matt Giuca wrote: > On 2014/10/30 17:04:56, grt wrote: > > it seems that this does something completely different than the "app_shim" for > > OSX in this directory. overloading the term like this seems like a mistake. is > > there a more appropriate name/location for this component? > > It's the same concept: a small shim to run an app because the OS won't let us > run Chrome directly, for whatever reason. > > The differences are: > - The Mac one needs to stay resident for the lifetime of the app, whereas the > Windows one can quit once the app is launched. > - The Mac one is used to give the app a dock icon and it's own presence in the > OS menu, as well as to provide a target for file associations. The Windows one > is just to provide a target for for associations. > > Those differences aren't enough to justify using a different name. They do > almost exactly the same thing and are both used for the same purpose. Ah, I misunderstood what the Mac one does. Thanks for clarifying. In that case, why not put both in app_shim.gypi with platform-specific conditionals inside for each target?
lgtm https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.cc (right): https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:23: // This program takes a full Windows command line as command-line arguments, and On 2014/10/30 06:39:05, Matt Giuca wrote: > OK I'm using the registry to get the Chrome binary path. We still allow > arbitrary command-line flags. This shouldn't be a problem on Chromium because > I've got another patch to get the Chromium binary path from the registry. (It > still needs to be installed, but it does work for Chromium now.) Awesome, thanks! https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.gypi (right): https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.gypi:22: 'msvs_settings': { On 2014/10/30 06:39:05, Matt Giuca wrote: > OK, I haven't really looked into this. Since it doesn't seem to be necessary > right now, I think I will go ahead without one and look into adding one > alongside the CL where we bundle the exe with Chrome. > > What is "shimming" in this context? API behaviour changes in different OS revisions and Windows uses the compatibility section of the manifest to figure out which OS version's behaviour to emulate. One of the emulation mechanisms used is AppCompat which inserts shims (which are just patches) to preserve old behaviour.
Patchset #11 (id:240001) has been deleted
Patchset #12 (id:280001) has been deleted
mgiuca@chromium.org changed reviewers: + jhawkins@chromium.org
jhawkins: Please review chrome.gyp. https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.gypi (right): https://codereview.chromium.org/179223003/diff/140001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.gypi:22: 'msvs_settings': { OK, I've added the manifest. Note: The way the build system is set up, ALL Windows binaries get the standard compatibility manifest regardless of whether they include their own (so this was not a concern). But I have added my own manifest that specifies "asInvoked" (copied from the other manifests around the place). https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.cc (right): https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:13: #include <windows.h> On 2014/10/30 17:04:56, grt wrote: > move above line 5 as per > http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Names_and_Ord.... Done. https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:46: int WINAPI WinMain(HINSTANCE instance, On 2014/10/30 17:04:56, grt wrote: > WinMain -> wWinMain Done. https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:75: if (!base::LaunchProcess(command_line, options, nullptr)) On 2014/10/30 17:04:56, grt wrote: > options -> base::LaunchOptions() Done. https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.cc:76: return kLaunchFailure; On 2014/10/30 17:04:56, grt wrote: > LOG(ERROR) here as on line 63? Done. https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.gypi (right): https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.gypi:10: 'target_name': 'app_shim_win', Discussed with Jack (owner of app_shim.gypi); the files do not share any code and making them work in the same file means extra complexity (checking for OS and excluding code), because there are variables required by the Mac one that aren't available on Windows. Jack: "I'd rather two readable .gypi files than one unreadable one." He's going to rename his file to app_shim_mac.gypi so there is no confusion. https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.gypi:13: '../..', Apparently not. Deleted.
lgtm https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... File chrome/app_shim/app_shim_win.gypi (right): https://codereview.chromium.org/179223003/diff/220001/chrome/app_shim/app_shi... chrome/app_shim/app_shim_win.gypi:10: 'target_name': 'app_shim_win', On 2014/11/04 04:38:17, Matt Giuca wrote: > Discussed with Jack (owner of app_shim.gypi); the files do not share any code > and making them work in the same file means extra complexity (checking for OS > and excluding code), because there are variables required by the Mac one that > aren't available on Windows. > > Jack: "I'd rather two readable .gypi files than one unreadable one." > > He's going to rename his file to app_shim_mac.gypi so there is no confusion. SGTM, thanks.
The CQ bit was checked by mgiuca@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/179223003/300001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
thestig@chromium.org changed reviewers: + thestig@chromium.org
chrome.gyp lgtm with a nit: https://codereview.chromium.org/179223003/diff/300001/chrome/chrome.gyp File chrome/chrome.gyp (right): https://codereview.chromium.org/179223003/diff/300001/chrome/chrome.gyp#newco... chrome/chrome.gyp:515: 'app_shim/app_shim_win.gypi', nit: alphabetical order please
https://codereview.chromium.org/179223003/diff/300001/chrome/chrome.gyp File chrome/chrome.gyp (right): https://codereview.chromium.org/179223003/diff/300001/chrome/chrome.gyp#newco... chrome/chrome.gyp:515: 'app_shim/app_shim_win.gypi', On 2014/11/04 23:23:44, Lei Zhang wrote: > nit: alphabetical order please Done.
The CQ bit was checked by mgiuca@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/179223003/320001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by mgiuca@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/179223003/320001
Message was sent while issue was closed.
Committed patchset #13 (id:320001)
Message was sent while issue was closed.
Patchset 13 (id:??) landed as https://crrev.com/31d809c90a47afe763ea6a027d811eae1707914f Cr-Commit-Position: refs/heads/master@{#302749} |