Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1769503002:
Add check for Java VM in Android's SysInfo::IsLowEndDevice() (Closed)
|
Created:
4 years, 9 months ago by Sami Väisänen Modified:
4 years, 8 months ago CC:
chromium-reviews, no sievers, vmpstr+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd check for Java VM in Android's SysInfo::IsLowEndDevice()
Android's SysInfo::IsLowEndDevice() calls into Java SysUtils
and thus expects to have a Java VM setup. However when using
command buffer as a standalone library VM doesn't exist.
BUG=skia:2992
Committed: https://crrev.com/2e4bd66751deee422cf15bd991b77b54a10b8278
Cr-Commit-Position: refs/heads/master@{#386343}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Removed Android specific IsLowEndDevice and enabled the generic IsLowEndDevice #Patch Set 3 : Reverted back to is vm check, updated comment #Messages
Total messages: 27 (6 generated)
Description was changed from ========== Add check for Java VM in Android's SysInfo::IsLowEndDevice() Add check for Java VM in Android IsLowEndDevice Android's SysInfo::IsLowEndDevice() calls into Java SysUtils and thus expects to have a Java VM setup. However when using command buffer as a standalone library VM doesn't exist. BUG=skia:2992 ========== to ========== Add check for Java VM in Android's SysInfo::IsLowEndDevice() Android's SysInfo::IsLowEndDevice() calls into Java SysUtils and thus expects to have a Java VM setup. However when using command buffer as a standalone library VM doesn't exist. BUG=skia:2992 ==========
svaisanen@nvidia.com changed reviewers: + yfriedman@chromium.org
When command buffer is used as a standalone library (through nanobench) there's no Java VM environment to setup. Hence the access to IsLowEndDevice() produces an assert. A similar problem was encountered before regardomg GPU information (gpu_info_collector_android.cc) and a similar if (have VM) check was added.
pardon my ignorance as I don't know anything about the tools you're referring to but how far up the stack do you need to make this work? Is this the only change needed or are there more? I worry that you're trying to do something that may not be feasible as in general we don't think about environments where java isn't availab.e https://codereview.chromium.org/1769503002/diff/1/base/sys_info_android.cc File base/sys_info_android.cc (right): https://codereview.chromium.org/1769503002/diff/1/base/sys_info_android.cc#ne... base/sys_info_android.cc:219: // When command buffer is compiled as a standalone library, the process I don't think this comment makes sense here. It's a low-level API and shouldn't refer to command buffer. You could rephrase as "May be use in some executables which don't have a Java environment"
On 2016/03/05 01:31:05, Yaron wrote: > pardon my ignorance as I don't know anything about the tools you're referring to > but how far up the stack do you need to make this work? Is this the only change > needed or are there more? I worry that you're trying to do something that may > not be feasible as in general we don't think about environments where java isn't > availab.e > The environment I'm referring to is part of the Skia tools. Nanobench is a perf measurement tool and it can be also used to create graphics contexts that run over the command buffer. However when such a configuration is used there's no Java environment since nanobench is just a native console application. If there's a better way or a place to do this please let me know. > https://codereview.chromium.org/1769503002/diff/1/base/sys_info_android.cc > File base/sys_info_android.cc (right): > > https://codereview.chromium.org/1769503002/diff/1/base/sys_info_android.cc#ne... > base/sys_info_android.cc:219: // When command buffer is compiled as a standalone > library, the process > I don't think this comment makes sense here. It's a low-level API and shouldn't > refer to command buffer. > > You could rephrase as "May be use in some executables which don't have a Java > environment" Sure.
On 2016/03/07 11:59:10, Sami Väisänen wrote: > On 2016/03/05 01:31:05, Yaron wrote: > > pardon my ignorance as I don't know anything about the tools you're referring > to > > but how far up the stack do you need to make this work? Is this the only > change > > needed or are there more? I worry that you're trying to do something that may > > not be feasible as in general we don't think about environments where java > isn't > > availab.e > > > > The environment I'm referring to is part of the Skia tools. Nanobench is a perf > measurement tool and it can be also used to create graphics contexts that run > over the command buffer. However when such a configuration is used there's no > Java environment since nanobench is just a native console application. If > there's a better way or a place to do this please let me know. What parts of chromium can these tools depend on? Is it only base/ ? Is this one CL sufficient to make these tools work or are there additional ones? > > > https://codereview.chromium.org/1769503002/diff/1/base/sys_info_android.cc > > File base/sys_info_android.cc (right): > > > > > https://codereview.chromium.org/1769503002/diff/1/base/sys_info_android.cc#ne... > > base/sys_info_android.cc:219: // When command buffer is compiled as a > standalone > > library, the process > > I don't think this comment makes sense here. It's a low-level API and > shouldn't > > refer to command buffer. > > > > You could rephrase as "May be use in some executables which don't have a Java > > environment" > > Sure.
On 2016/03/07 14:17:03, Yaron wrote: > On 2016/03/07 11:59:10, Sami Väisänen wrote: > > On 2016/03/05 01:31:05, Yaron wrote: > > > pardon my ignorance as I don't know anything about the tools you're > referring > > to > > > but how far up the stack do you need to make this work? Is this the only > > change > > > needed or are there more? I worry that you're trying to do something that > may > > > not be feasible as in general we don't think about environments where java > > isn't > > > availab.e > > > > > > > The environment I'm referring to is part of the Skia tools. Nanobench is a > perf > > measurement tool and it can be also used to create graphics contexts that run > > over the command buffer. However when such a configuration is used there's no > > Java environment since nanobench is just a native console application. If > > there's a better way or a place to do this please let me know. > > What parts of chromium can these tools depend on? Is it only base/ ? Is this one > CL sufficient to make these tools work or are there additional ones? > > There's no build time dependency but only runtime dependency through dynamically loading the command_buffer_gles2 library which itself has dependencies to base and ui/gl (at least). Currently this is the only change that I've needed to do in order to make command buffer work again in a process (nanobench) without JV. These issues come up intermittently as the chromium software evolves. > > > https://codereview.chromium.org/1769503002/diff/1/base/sys_info_android.cc > > > File base/sys_info_android.cc (right): > > > > > > > > > https://codereview.chromium.org/1769503002/diff/1/base/sys_info_android.cc#ne... > > > base/sys_info_android.cc:219: // When command buffer is compiled as a > > standalone > > > library, the process > > > I don't think this comment makes sense here. It's a low-level API and > > shouldn't > > > refer to command buffer. > > > > > > You could rephrase as "May be use in some executables which don't have a > Java > > > environment" > > > > Sure.
yfriedman@chromium.org changed reviewers: + aelias@chromium.org
Right - I'm hoping to avoid having to randomly sprinkle these throughout the codebase as the code gets refactored and am wondering if there was a principled way of doing it. +aelias to comment on whether he thinks it's viable to keep these areas working without java (as best effort, of course). if so then it should be fine
Hmm, the round-trip to Java is really unnecessary, all the Java code does is read /proc/meminfo which could just as well be done from C++. Indeed there already exists a slightly different Linux implementation which may well have the same result, so it might be sufficient just to delete these functions from sys_info_android.cc and un-ifdef the ones in sys_info.cc. Could you give that a try?
On 2016/03/10 05:55:30, aelias wrote: > Hmm, the round-trip to Java is really unnecessary, all the Java code does is > read /proc/meminfo which could just as well be done from C++. Indeed there > already exists a slightly different Linux implementation which may well have the > same result, so it might be sufficient just to delete these functions from > sys_info_android.cc and un-ifdef the ones in sys_info.cc. Could you give that a > try? I'll try it out.
On 2016/03/11 08:58:29, Sami Väisänen wrote: > On 2016/03/10 05:55:30, aelias wrote: > > Hmm, the round-trip to Java is really unnecessary, all the Java code does is > > read /proc/meminfo which could just as well be done from C++. Indeed there > > already exists a slightly different Linux implementation which may well have > the > > same result, so it might be sufficient just to delete these functions from > > sys_info_android.cc and un-ifdef the ones in sys_info.cc. Could you give that > a > > try? > > I'll try it out. This appears to work. There also exists base/android/SysUtils.h/.cc that might be unneeded. At least the native code is no longer calling this. However I'm not sure about whether there are some Java components that are calling it so leaving it as is for now.
On 2016/03/14 09:24:24, Sami Väisänen wrote: > On 2016/03/11 08:58:29, Sami Väisänen wrote: > > On 2016/03/10 05:55:30, aelias wrote: > > > Hmm, the round-trip to Java is really unnecessary, all the Java code does is > > > read /proc/meminfo which could just as well be done from C++. Indeed there > > > already exists a slightly different Linux implementation which may well have > > the > > > same result, so it might be sufficient just to delete these functions from > > > sys_info_android.cc and un-ifdef the ones in sys_info.cc. Could you give > that > > a > > > try? > > > > I'll try it out. > > This appears to work. > There also exists base/android/SysUtils.h/.cc that might be unneeded. At least > the native code is no longer calling this. However I'm not sure about whether > there are some Java components that are calling it so leaving it as is for now. There aren't java components that could call it. It's strictly a C++ -> Java API The unfortunate thing is that the java side isn't removed so the code is now duplicated and in the worst case (e.g. field trial active) we could get different answers depending on which side of JNI you're on. It's also not possible to make the Java side depend on your native impl since it's called before native is even loaded. :(
On 2016/03/14 20:31:01, Yaron wrote: > On 2016/03/14 09:24:24, Sami Väisänen wrote: > > On 2016/03/11 08:58:29, Sami Väisänen wrote: > > > On 2016/03/10 05:55:30, aelias wrote: > > > > Hmm, the round-trip to Java is really unnecessary, all the Java code does > is > > > > read /proc/meminfo which could just as well be done from C++. Indeed > there > > > > already exists a slightly different Linux implementation which may well > have > > > the > > > > same result, so it might be sufficient just to delete these functions from > > > > sys_info_android.cc and un-ifdef the ones in sys_info.cc. Could you give > > that > > > a > > > > try? > > > > > > I'll try it out. > > > > This appears to work. > > There also exists base/android/SysUtils.h/.cc that might be unneeded. At least > > the native code is no longer calling this. However I'm not sure about whether > > there are some Java components that are calling it so leaving it as is for > now. > > There aren't java components that could call it. It's strictly a C++ -> Java API > > The unfortunate thing is that the java side isn't removed so the code is now > duplicated and in the worst case (e.g. field trial active) we could get > different answers depending on which side of JNI you're on. It's also not > possible to make the Java side depend on your native impl since it's called > before native is even loaded. :( So what does this mean?
On 2016/03/15 08:37:01, Sami Väisänen wrote: > On 2016/03/14 20:31:01, Yaron wrote: > > On 2016/03/14 09:24:24, Sami Väisänen wrote: > > > On 2016/03/11 08:58:29, Sami Väisänen wrote: > > > > On 2016/03/10 05:55:30, aelias wrote: > > > > > Hmm, the round-trip to Java is really unnecessary, all the Java code > does > > is > > > > > read /proc/meminfo which could just as well be done from C++. Indeed > > there > > > > > already exists a slightly different Linux implementation which may well > > have > > > > the > > > > > same result, so it might be sufficient just to delete these functions > from > > > > > sys_info_android.cc and un-ifdef the ones in sys_info.cc. Could you > give > > > that > > > > a > > > > > try? > > > > > > > > I'll try it out. > > > > > > This appears to work. > > > There also exists base/android/SysUtils.h/.cc that might be unneeded. At > least > > > the native code is no longer calling this. However I'm not sure about > whether > > > there are some Java components that are calling it so leaving it as is for > > now. > > > > There aren't java components that could call it. It's strictly a C++ -> Java > API > > > > The unfortunate thing is that the java side isn't removed so the code is now > > duplicated and in the worst case (e.g. field trial active) we could get > > different answers depending on which side of JNI you're on. It's also not > > possible to make the Java side depend on your native impl since it's called > > before native is even loaded. :( > > So what does this mean? well, i guess the IsVM check only really affects your use case which is specialized and not user-facing. That breaking from time to time is probably less of an issue than chrome actually having user-facing issues. I originally added aelias@ to see if your general approach was viable or if we'd expect it to frequently break. It sounds like it can break but is feasible so I'd prefer it
On 2016/03/16 17:43:35, Yaron wrote: > On 2016/03/15 08:37:01, Sami Väisänen wrote: > > On 2016/03/14 20:31:01, Yaron wrote: > > > On 2016/03/14 09:24:24, Sami Väisänen wrote: > > > > On 2016/03/11 08:58:29, Sami Väisänen wrote: > > > > > On 2016/03/10 05:55:30, aelias wrote: > > > > > > Hmm, the round-trip to Java is really unnecessary, all the Java code > > does > > > is > > > > > > read /proc/meminfo which could just as well be done from C++. Indeed > > > there > > > > > > already exists a slightly different Linux implementation which may > well > > > have > > > > > the > > > > > > same result, so it might be sufficient just to delete these functions > > from > > > > > > sys_info_android.cc and un-ifdef the ones in sys_info.cc. Could you > > give > > > > that > > > > > a > > > > > > try? > > > > > > > > > > I'll try it out. > > > > > > > > This appears to work. > > > > There also exists base/android/SysUtils.h/.cc that might be unneeded. At > > least > > > > the native code is no longer calling this. However I'm not sure about > > whether > > > > there are some Java components that are calling it so leaving it as is for > > > now. > > > > > > There aren't java components that could call it. It's strictly a C++ -> Java > > API > > > > > > The unfortunate thing is that the java side isn't removed so the code is now > > > duplicated and in the worst case (e.g. field trial active) we could get > > > different answers depending on which side of JNI you're on. It's also not > > > possible to make the Java side depend on your native impl since it's called > > > before native is even loaded. :( > > > > So what does this mean? > > well, i guess the IsVM check only really affects your use case which is > specialized and not user-facing. That breaking from time to time is probably > less of an issue than chrome actually having user-facing issues. I originally > added aelias@ to see if your general approach was viable or if we'd expect it to > frequently break. It sounds like it can break but is feasible so I'd prefer it Hmm, I see what you mean now. SysUtils.java implements the same logic as sys_info.cc and the code is indeed duplicated. What would be the steps required to make Java call the native implementation instead of duplicating the code there? I think it would make most sense to have Java call into native and have the code in a single place instead.
On 2016/03/17 10:04:42, Sami Väisänen wrote: > On 2016/03/16 17:43:35, Yaron wrote: > > On 2016/03/15 08:37:01, Sami Väisänen wrote: > > > On 2016/03/14 20:31:01, Yaron wrote: > > > > On 2016/03/14 09:24:24, Sami Väisänen wrote: > > > > > On 2016/03/11 08:58:29, Sami Väisänen wrote: > > > > > > On 2016/03/10 05:55:30, aelias wrote: > > > > > > > Hmm, the round-trip to Java is really unnecessary, all the Java code > > > does > > > > is > > > > > > > read /proc/meminfo which could just as well be done from C++. > Indeed > > > > there > > > > > > > already exists a slightly different Linux implementation which may > > well > > > > have > > > > > > the > > > > > > > same result, so it might be sufficient just to delete these > functions > > > from > > > > > > > sys_info_android.cc and un-ifdef the ones in sys_info.cc. Could you > > > give > > > > > that > > > > > > a > > > > > > > try? > > > > > > > > > > > > I'll try it out. > > > > > > > > > > This appears to work. > > > > > There also exists base/android/SysUtils.h/.cc that might be unneeded. At > > > least > > > > > the native code is no longer calling this. However I'm not sure about > > > whether > > > > > there are some Java components that are calling it so leaving it as is > for > > > > now. > > > > > > > > There aren't java components that could call it. It's strictly a C++ -> > Java > > > API > > > > > > > > The unfortunate thing is that the java side isn't removed so the code is > now > > > > duplicated and in the worst case (e.g. field trial active) we could get > > > > different answers depending on which side of JNI you're on. It's also not > > > > possible to make the Java side depend on your native impl since it's > called > > > > before native is even loaded. :( > > > > > > So what does this mean? > > > > well, i guess the IsVM check only really affects your use case which is > > specialized and not user-facing. That breaking from time to time is probably > > less of an issue than chrome actually having user-facing issues. I originally > > added aelias@ to see if your general approach was viable or if we'd expect it > to > > frequently break. It sounds like it can break but is feasible so I'd prefer it > > Hmm, I see what you mean now. SysUtils.java implements the same logic as > sys_info.cc and the code is indeed duplicated. What would be the steps required > to make Java call the native implementation instead of duplicating the code > there? I think it would make most sense to have Java call into native and have > the code in a single place instead. It's not possible to have the Java depend on native in this case. At least not without allowing some amount of time with code duplication. On Android, chrome is a java app that loads native code but it exists before native is loaded. Some characteristics of loading the native library actually depend on whether its a low-end device so you have a chicken and egg problem (see LegacyLinker#ensureInitializedLocked)
On 2016/03/18 14:49:00, Yaron wrote: > On 2016/03/17 10:04:42, Sami Väisänen wrote: > > On 2016/03/16 17:43:35, Yaron wrote: > > > On 2016/03/15 08:37:01, Sami Väisänen wrote: > > > > On 2016/03/14 20:31:01, Yaron wrote: > > > > > On 2016/03/14 09:24:24, Sami Väisänen wrote: > > > > > > On 2016/03/11 08:58:29, Sami Väisänen wrote: > > > > > > > On 2016/03/10 05:55:30, aelias wrote: > > > > > > > > Hmm, the round-trip to Java is really unnecessary, all the Java > code > > > > does > > > > > is > > > > > > > > read /proc/meminfo which could just as well be done from C++. > > Indeed > > > > > there > > > > > > > > already exists a slightly different Linux implementation which may > > > well > > > > > have > > > > > > > the > > > > > > > > same result, so it might be sufficient just to delete these > > functions > > > > from > > > > > > > > sys_info_android.cc and un-ifdef the ones in sys_info.cc. Could > you > > > > give > > > > > > that > > > > > > > a > > > > > > > > try? > > > > > > > > > > > > > > I'll try it out. > > > > > > > > > > > > This appears to work. > > > > > > There also exists base/android/SysUtils.h/.cc that might be unneeded. > At > > > > least > > > > > > the native code is no longer calling this. However I'm not sure about > > > > whether > > > > > > there are some Java components that are calling it so leaving it as is > > for > > > > > now. > > > > > > > > > > There aren't java components that could call it. It's strictly a C++ -> > > Java > > > > API > > > > > > > > > > The unfortunate thing is that the java side isn't removed so the code is > > now > > > > > duplicated and in the worst case (e.g. field trial active) we could get > > > > > different answers depending on which side of JNI you're on. It's also > not > > > > > possible to make the Java side depend on your native impl since it's > > called > > > > > before native is even loaded. :( > > > > > > > > So what does this mean? > > > > > > well, i guess the IsVM check only really affects your use case which is > > > specialized and not user-facing. That breaking from time to time is probably > > > less of an issue than chrome actually having user-facing issues. I > originally > > > added aelias@ to see if your general approach was viable or if we'd expect > it > > to > > > frequently break. It sounds like it can break but is feasible so I'd prefer > it > > > > Hmm, I see what you mean now. SysUtils.java implements the same logic as > > sys_info.cc and the code is indeed duplicated. What would be the steps > required > > to make Java call the native implementation instead of duplicating the code > > there? I think it would make most sense to have Java call into native and have > > the code in a single place instead. > > It's not possible to have the Java depend on native in this case. At least not > without allowing some amount of time with code duplication. On Android, chrome > is a java app that loads native code but it exists before native is loaded. Some > characteristics of loading the native library actually depend on whether its a > low-end device so you have a chicken and egg problem (see > LegacyLinker#ensureInitializedLocked) Ok, So I will revert back to the 1st patch and have the check for VM.
On 2016/03/21 07:07:32, Sami Väisänen wrote: > On 2016/03/18 14:49:00, Yaron wrote: > > On 2016/03/17 10:04:42, Sami Väisänen wrote: > > > On 2016/03/16 17:43:35, Yaron wrote: > > > > On 2016/03/15 08:37:01, Sami Väisänen wrote: > > > > > On 2016/03/14 20:31:01, Yaron wrote: > > > > > > On 2016/03/14 09:24:24, Sami Väisänen wrote: > > > > > > > On 2016/03/11 08:58:29, Sami Väisänen wrote: > > > > > > > > On 2016/03/10 05:55:30, aelias wrote: > > > > > > > > > Hmm, the round-trip to Java is really unnecessary, all the Java > > code > > > > > does > > > > > > is > > > > > > > > > read /proc/meminfo which could just as well be done from C++. > > > Indeed > > > > > > there > > > > > > > > > already exists a slightly different Linux implementation which > may > > > > well > > > > > > have > > > > > > > > the > > > > > > > > > same result, so it might be sufficient just to delete these > > > functions > > > > > from > > > > > > > > > sys_info_android.cc and un-ifdef the ones in sys_info.cc. Could > > you > > > > > give > > > > > > > that > > > > > > > > a > > > > > > > > > try? > > > > > > > > > > > > > > > > I'll try it out. > > > > > > > > > > > > > > This appears to work. > > > > > > > There also exists base/android/SysUtils.h/.cc that might be > unneeded. > > At > > > > > least > > > > > > > the native code is no longer calling this. However I'm not sure > about > > > > > whether > > > > > > > there are some Java components that are calling it so leaving it as > is > > > for > > > > > > now. > > > > > > > > > > > > There aren't java components that could call it. It's strictly a C++ > -> > > > Java > > > > > API > > > > > > > > > > > > The unfortunate thing is that the java side isn't removed so the code > is > > > now > > > > > > duplicated and in the worst case (e.g. field trial active) we could > get > > > > > > different answers depending on which side of JNI you're on. It's also > > not > > > > > > possible to make the Java side depend on your native impl since it's > > > called > > > > > > before native is even loaded. :( > > > > > > > > > > So what does this mean? > > > > > > > > well, i guess the IsVM check only really affects your use case which is > > > > specialized and not user-facing. That breaking from time to time is > probably > > > > less of an issue than chrome actually having user-facing issues. I > > originally > > > > added aelias@ to see if your general approach was viable or if we'd expect > > it > > > to > > > > frequently break. It sounds like it can break but is feasible so I'd > prefer > > it > > > > > > Hmm, I see what you mean now. SysUtils.java implements the same logic as > > > sys_info.cc and the code is indeed duplicated. What would be the steps > > required > > > to make Java call the native implementation instead of duplicating the code > > > there? I think it would make most sense to have Java call into native and > have > > > the code in a single place instead. > > > > It's not possible to have the Java depend on native in this case. At least not > > without allowing some amount of time with code duplication. On Android, chrome > > is a java app that loads native code but it exists before native is loaded. > Some > > characteristics of loading the native library actually depend on whether its a > > low-end device so you have a chicken and egg problem (see > > LegacyLinker#ensureInitializedLocked) > > Ok, So I will revert back to the 1st patch and have the check for VM. Reverted back to having the check for IsVM and updated the comment to include some of the rational in this disuccion. PTAL
lgtm
lgtm
The CQ bit was checked by svaisanen@nvidia.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1769503002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1769503002/40001
Message was sent while issue was closed.
Description was changed from ========== Add check for Java VM in Android's SysInfo::IsLowEndDevice() Android's SysInfo::IsLowEndDevice() calls into Java SysUtils and thus expects to have a Java VM setup. However when using command buffer as a standalone library VM doesn't exist. BUG=skia:2992 ========== to ========== Add check for Java VM in Android's SysInfo::IsLowEndDevice() Android's SysInfo::IsLowEndDevice() calls into Java SysUtils and thus expects to have a Java VM setup. However when using command buffer as a standalone library VM doesn't exist. BUG=skia:2992 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Add check for Java VM in Android's SysInfo::IsLowEndDevice() Android's SysInfo::IsLowEndDevice() calls into Java SysUtils and thus expects to have a Java VM setup. However when using command buffer as a standalone library VM doesn't exist. BUG=skia:2992 ========== to ========== Add check for Java VM in Android's SysInfo::IsLowEndDevice() Android's SysInfo::IsLowEndDevice() calls into Java SysUtils and thus expects to have a Java VM setup. However when using command buffer as a standalone library VM doesn't exist. BUG=skia:2992 Committed: https://crrev.com/2e4bd66751deee422cf15bd991b77b54a10b8278 Cr-Commit-Position: refs/heads/master@{#386343} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/2e4bd66751deee422cf15bd991b77b54a10b8278 Cr-Commit-Position: refs/heads/master@{#386343} |
