|
|
Created:
6 years, 4 months ago by Greg Levin Modified:
6 years, 2 months ago CC:
chromium-apps-reviews_chromium.org, chromium-reviews, darin-cc_chromium.org, davemoore+watch_chromium.org, extensions-reviews_chromium.org, jam, nkostylev+watch_chromium.org, oshima+watch_chromium.org, stevenjb+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Project:
chromium Visibility:
Public. |
DescriptionHandle large wallpaper decoding in utility process:
Images over 32M pixels exceed IPC message limit size. In ChromeContentUtilityClient::DecodeImage(), cut image dimensions in half to minimize quality loss and allow decoded image to be returned via IPC message.
BUG=366825
TEST=ChromeContentUtilityClientTest.DecodeImageSizeLimit; Set ChromeOS wallpaper using JPGs attached to bug
Committed: https://crrev.com/d9d0e601c4c45ec8f2fb1d148e68458d9baa4e24
Cr-Commit-Position: refs/heads/master@{#296407}
Committed: https://crrev.com/b18f4aa7717e44c208588b56f534e5e6ab7cd69e
Cr-Commit-Position: refs/heads/master@{#296939}
Patch Set 1 : Resize image in utility process to fit IPC message size limit #
Total comments: 26
Patch Set 2 : Address reviewer comments, rework DecodeImage() #
Total comments: 2
Patch Set 3 : Add comment #Patch Set 4 : Fix unit test for Android #Patch Set 5 : Add comment about Issue 416916 #Patch Set 6 : Customize & reduce max_msg_size for testing #
Total comments: 6
Patch Set 7 : Rename function and variable #
Total comments: 2
Patch Set 8 : Add space #Messages
Total messages: 64 (18 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
glevin@chromium.org changed reviewers: + dcheng@chromium.org, thakis@chromium.org, yoz@chromium.org
Noogler's first substantial CL, all feedback appreciated. * Would other callers of ImageDecoder() benefit from this change? ChromeUtilityMsg_DecodeImage currently fails silently on overlarge images w/o shrink_to_fit flag set. * Copied CreateJPEGImage() in unit test file from wallpaper_manager_test_utils.cc to avoid #include module violation. * Separated out DecodeImageAndSend() so I could unit test DecodeImage(). * Size calc in IsMessageTooBig() differs slightly from getSafeSize64() in SkImageInfo.h, but is simpler and seems sufficient. * Really large (~15000x10000) images slow UI responsiveness in wallpaper picker. My fix doesn't address this.
On 2014/08/27 15:45:00, Greg Levin wrote: > Noogler's first substantial CL, all feedback appreciated. > > * Would other callers of ImageDecoder() benefit from this change? > ChromeUtilityMsg_DecodeImage currently fails silently on overlarge images w/o > shrink_to_fit flag set. > * Copied CreateJPEGImage() in unit test file from > wallpaper_manager_test_utils.cc to avoid #include module violation. > * Separated out DecodeImageAndSend() so I could unit test DecodeImage(). > * Size calc in IsMessageTooBig() differs slightly from getSafeSize64() in > SkImageInfo.h, but is simpler and seems sufficient. > * Really large (~15000x10000) images slow UI responsiveness in wallpaper picker. > My fix doesn't address this. When sending to multiple reviewers, it helps to specify which files you would like each of them to review. Also, your other 2 reviewers might be away, according to their Chromium code review handles; try asking around to find alternates?
> When sending to multiple reviewers, it helps to specify which files you would > like each of them to review. Also, your other 2 reviewers might be away, > according to their Chromium code review handles; try asking around to find > alternates? Ah, ok, thanks... I was just going off upload's suggested reviewers. Looking at owners, yoz - wallpaper_function_base.cc, webstore_install_helper.cc, extensions_handler.cc thakis - image_decoder.cc/h, chrome_tests_unit.gypi dcheng - chrome_utility_messages.h, chrome_content_utility_client.cc/h, chrome_content_utility_client_unittest.cc Will try different reviewers if I don't get a response soon.
The usual way is to get someone who's familiar with the feature you're trying to do to review everything, and once it's in good shape you add owners who ideally can quickly approve. https://codereview.chromium.org/482163002/diff/60001/chrome/browser/image_dec... File chrome/browser/image_decoder.cc (right): https://codereview.chromium.org/482163002/diff/60001/chrome/browser/image_dec... chrome/browser/image_decoder.cc:23: shrink_to_fit_(false) { Is this ever set?
https://codereview.chromium.org/482163002/diff/60001/chrome/browser/image_dec... File chrome/browser/image_decoder.cc (right): https://codereview.chromium.org/482163002/diff/60001/chrome/browser/image_dec... chrome/browser/image_decoder.cc:23: shrink_to_fit_(false) { On 2014/08/27 22:32:15, Nico (hiding) wrote: > Is this ever set? nvm, there's a setter in the header. Sorry for the silly question.
extensions changes LGTM
glevin@chromium.org changed reviewers: + bshe@chromium.org, tsepez@chromium.org - dcheng@chromium.org
> The usual way is to get someone who's familiar with the feature you're trying to > do to review everything [...] I've been working with bshe@ on this for awhile, but I should've had him as a formal reviewer from the start. Fixed.
The messages are OK, but if you can avoid multiplications, etc. you can dodge all sorts of issues with overflows, etc. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... File chrome/utility/chrome_content_utility_client.cc (right): https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:193: // If decoded image is too large for IPC message, shrink it by halves looks like using SkBitmap::computeSize64() would avoid these calculations. And if IsMessageTooBig took a size_t (or an int64_t to be consistent with Sk's unfortunate use of signed types), it could avoid multiplying, too ... https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:197: do { A while loop may get rid of having to call IsMessageTooBig from two places in this function. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:198: width = width/2; ... having done the above, then in this loop, you'd divide SkBitmap::computeSize64() by 4 on each pass (since you're havling in each dimension), keeping track of iterations ... https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:201: decoded_image = skia::ImageOperations::Resize( Lastly, you can extract and divide width, height appropriately.
https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... File chrome/utility/chrome_content_utility_client_unittest.cc (right): https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client_unittest.cc:57: how about 4x just to run your loop more than once?
this is great. I just have a few nits and optional suggestions. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... File chrome/utility/chrome_content_utility_client.cc (right): https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:191: encoded_data.size()); nit: indent https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:193: // If decoded image is too large for IPC message, shrink it by halves Ideally, I think we should shrink to best fit. Not just shrink by halves. This can avoid the over shrunk problem when the image is just slightly over the limit. Do you mind to leave a TODO. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:199: height = height/2; nit: width /= 2 or width = width / 2;. same for height. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:213: Add an error message for over IPC limit case would be nice. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... File chrome/utility/chrome_content_utility_client.h (right): https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.h:36: bool shrink_to_fit); nit: indent is off https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... File chrome/utility/chrome_content_utility_client_unittest.cc (right): https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client_unittest.cc:11: bool CreateJPEGImage(int width, nit: wrap this function in a anonymous namespace. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client_unittest.cc:56: static_cast<int64_t>(IPC::Channel::kMaximumMessageSize)); nit: indent
> [...] if you can avoid multiplications, etc. you can dodge > all sorts of issues with overflows, etc. > [...] looks like using SkBitmap::computeSize64() would avoid these calculations. > [...] Lastly, you can extract and divide width, height appropriately. Thanks for the suggestions... definite improvement! I've reworked DecodeImage(), hopefully cleaner now. > Add an error message for over IPC limit case would be nice. Error logged. I've also modified DecodeImage() so that, if the decoded image is too big but shrink_to_fit isn't set, it returns an empty image. This will cause DecodeImageAndSend() to send ChromeUtilityHostMsg_DecodeImage_Failed, so that at least the IPC message won't fail silently as it did before. > how about 4x just to run your loop more than once? Done. Also added test for returned blank image when image is too big and shrink_to_fit isn't set.
Addressed comments, please have a look. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... File chrome/utility/chrome_content_utility_client.cc (right): https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:191: encoded_data.size()); On 2014/08/29 19:56:26, bshe wrote: > nit: indent Done. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:193: // If decoded image is too large for IPC message, shrink it by halves On 2014/08/29 19:56:26, bshe wrote: > Ideally, I think we should shrink to best fit. Not just shrink by halves. > This can avoid the over shrunk problem when the image is just slightly over the > limit. Do you mind to leave a TODO. Discussed off-line, decided shrinking by halves was okay (right?). Added comment to code. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:193: // If decoded image is too large for IPC message, shrink it by halves On 2014/08/29 19:47:13, Tom Sepez wrote: > looks like using SkBitmap::computeSize64() would avoid these calculations. And > if IsMessageTooBig took a size_t (or an int64_t to be consistent with Sk's > unfortunate use of signed types), it could avoid multiplying, too ... Rewrote size checking logic, hopefully addressed all concerns here https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:197: do { On 2014/08/29 19:47:13, Tom Sepez wrote: > A while loop may get rid of having to call IsMessageTooBig from two places in > this function. Done. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:198: width = width/2; On 2014/08/29 19:47:13, Tom Sepez wrote: > ... having done the above, then in this loop, you'd divide > SkBitmap::computeSize64() by 4 on each pass (since you're havling in each > dimension), keeping track of iterations ... Done. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:199: height = height/2; On 2014/08/29 19:56:26, bshe wrote: > nit: width /= 2 or width = width / 2;. same for height. Replaced division with bit shifts https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:201: decoded_image = skia::ImageOperations::Resize( On 2014/08/29 19:47:13, Tom Sepez wrote: > Lastly, you can extract and divide width, height appropriately. Done. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.cc:213: On 2014/08/29 19:56:26, bshe wrote: > Add an error message for over IPC limit case would be nice. Improved error handling and LOG'ed error https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... File chrome/utility/chrome_content_utility_client.h (right): https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client.h:36: bool shrink_to_fit); On 2014/08/29 19:56:26, bshe wrote: > nit: indent is off Done. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... File chrome/utility/chrome_content_utility_client_unittest.cc (right): https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client_unittest.cc:11: bool CreateJPEGImage(int width, On 2014/08/29 19:56:26, bshe wrote: > nit: wrap this function in a anonymous namespace. Done. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client_unittest.cc:56: static_cast<int64_t>(IPC::Channel::kMaximumMessageSize)); On 2014/08/29 19:56:26, bshe wrote: > nit: indent Done. https://codereview.chromium.org/482163002/diff/60001/chrome/utility/chrome_co... chrome/utility/chrome_content_utility_client_unittest.cc:57: On 2014/08/29 19:51:09, Tom Sepez wrote: > how about 4x just to run your loop more than once? Done. Also added check for "shrink_to_fit == false" case.
lgtm
On 2014/09/04 17:22:06, Tom Sepez wrote: > lgtm lgtm
glevin@chromium.org changed reviewers: + sky@chromium.org - thakis@chromium.org
LGTM with a description of your new field https://codereview.chromium.org/482163002/diff/80001/chrome/browser/image_dec... File chrome/browser/image_decoder.h (right): https://codereview.chromium.org/482163002/diff/80001/chrome/browser/image_dec... chrome/browser/image_decoder.h:75: bool shrink_to_fit_; Add a description of this.
The CQ bit was checked by skuhne@chromium.org
The CQ bit was unchecked by skuhne@chromium.org
Added comment, sending to skuhne https://codereview.chromium.org/482163002/diff/80001/chrome/browser/image_dec... File chrome/browser/image_decoder.h (right): https://codereview.chromium.org/482163002/diff/80001/chrome/browser/image_dec... chrome/browser/image_decoder.h:75: bool shrink_to_fit_; On 2014/09/09 20:19:11, sky wrote: > Add a description of this. Done.
The CQ bit was checked by skuhne@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/482163002/100001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_dbg_tests_recipe on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
dcheng@chromium.org changed reviewers: + dcheng@chromium.org
It seems unfortunate that we have different ways of solving this problem for different parts of the code. For supporting copying images, we actually copy the bitmap over shared memory. But for this, we try to resize the image to fit in the constraints. Would it make sense to try to use a common strategy for both?
Patchset #6 (id:160001) has been deleted
Patchset #5 (id:140001) has been deleted
Patchset #4 (id:120001) has been deleted
Trybot "android_dbg_tests_recipe" failed. Deep in content::DecodeImage(), Android was shrinking the image (for general memory conservation) before my code got to it (see ImageDecoder::create() and BlinkPlatformImpl::maxDecodedImageBytes() for details). This caused dimensions that differed from what the test expected. Since there is no way to predict the Android-shrunk image size, I disabled most of the testing on OS_ANDROID, leaving only the test for IPC message size conformity. Tom, since you're the owner for /chrome/utility/, could you take a quick look at these changes? Thanks! > It seems unfortunate that we have different ways of solving > this problem for different parts of the code. > > For supporting copying images, we actually copy the bitmap > over shared memory. But for this, we try to resize the > image to fit in the constraints. Would it make sense to > try to use a common strategy for both? I discussed both options with a few people in the office here, and the consensus seemed to be that either approach was fine. I have no opinion on the subject, as I'm relatively new and don't know the code base well. Does anyone else here have any thoughts on this?
On 2014/09/18 at 18:29:55, glevin wrote: > Trybot "android_dbg_tests_recipe" failed. Deep in content::DecodeImage(), Android was shrinking the image (for general memory conservation) before my code got to it (see ImageDecoder::create() and BlinkPlatformImpl::maxDecodedImageBytes() for details). This caused dimensions that differed from what the test expected. Since there is no way to predict the Android-shrunk image size, I disabled most of the testing on OS_ANDROID, leaving only the test for IPC message size conformity. > > Tom, since you're the owner for /chrome/utility/, could you take a quick look at these changes? Thanks! > > > It seems unfortunate that we have different ways of solving > > this problem for different parts of the code. > > > > For supporting copying images, we actually copy the bitmap > > over shared memory. But for this, we try to resize the > > image to fit in the constraints. Would it make sense to > > try to use a common strategy for both? > > I discussed both options with a few people in the office here, and the consensus seemed to be that either approach was fine. I have no opinion on the subject, as I'm relatively new and don't know the code base well. Does anyone else here have any thoughts on this? It just strikes me as rather leaky that we have to explicitly calculate the size of the bitmap and account for the overhead and make sure it fits under the IPC message limit. Keeping in mind that I'm clearly biased here, I'm refactoring the clipboard IPC bits in https://codereview.chromium.org/574273002 to separate the IPC handling from the clipboard-specific details. I don't think the shared memory bitmap transport is going anywhere, so I think it wouldn't be too much work to add an IPC::ParamTraits and a wrapper type in a followup patch for serializing unreasonably large bitmaps.
lgtm (didn't realize that I got on the reviewers list by trying to land the patch for him)
I'll discuss with dcheng when he has time what we should do.
As discussed with dcheng: Utility functions should do what they are supposed to do: Decode images to their full size. If that is not possible, the function should fail. However - there are cameras out there which can produce images which bust our limits. Furthermore the resources required to read such an image are substantial against 2GB main memory chrome books. Last but not least - fully decoded image cannot be shown on our monitors soon anyways. So we will have to draw a line somewhere - and this can be seen as a temporary bump of the limit in a useful way. As such we should land this patch and create an issue to fix this the proper way later on.
lgtm
lgtm
The CQ bit was checked by skuhne@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/482163002/190001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_swarming on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by skuhne@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/482163002/190001
Message was sent while issue was closed.
Committed patchset #5 (id:190001) as 4735e48a27fc303706197d626d19febd06c0375c
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/d9d0e601c4c45ec8f2fb1d148e68458d9baa4e24 Cr-Commit-Position: refs/heads/master@{#296407}
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:190001) has been created in https://codereview.chromium.org/600913002/ by jiayl@chromium.org. The reason for reverting is: Speculative revert for unit_tests failures: http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%2....
Message was sent while issue was closed.
Patchset #6 (id:210001) has been deleted
Message was sent while issue was closed.
Patchset #6 (id:230001) has been deleted
Message was sent while issue was closed.
An image that needed 4x reduction to fit kMaximumMessageSize was about 14000 x 9400, and test was timing out. Now allow test to modify max_message_size for testing purposes. Please have a quick look, if you're following this issue.
Message was sent while issue was closed.
This should do the trick! Only a few more nits... https://codereview.chromium.org/482163002/diff/250001/chrome/utility/chrome_c... File chrome/utility/chrome_content_utility_client.h (right): https://codereview.chromium.org/482163002/diff/250001/chrome/utility/chrome_c... chrome/utility/chrome_content_utility_client.h:40: static void set_max_message_size(int64_t max_message_size) { set_max_ipc_message_size_for_test(..) https://codereview.chromium.org/482163002/diff/250001/chrome/utility/chrome_c... chrome/utility/chrome_content_utility_client.h:81: std::set<int> message_id_whitelist_; The comment should be a line above and states something like: // The maximum IPC message size which can be overridden for testing purposes. https://codereview.chromium.org/482163002/diff/250001/chrome/utility/chrome_c... File chrome/utility/chrome_content_utility_client_unittest.cc (right): https://codereview.chromium.org/482163002/diff/250001/chrome/utility/chrome_c... chrome/utility/chrome_content_utility_client_unittest.cc:44: size_t max_message_size = IPC::Channel::kMaximumMessageSize / 1024; const size_t ..kTestMessageSize = ..
Message was sent while issue was closed.
https://codereview.chromium.org/482163002/diff/250001/chrome/utility/chrome_c... File chrome/utility/chrome_content_utility_client.h (right): https://codereview.chromium.org/482163002/diff/250001/chrome/utility/chrome_c... chrome/utility/chrome_content_utility_client.h:40: static void set_max_message_size(int64_t max_message_size) { On 2014/09/25 02:15:03, Mr4D wrote: > set_max_ipc_message_size_for_test(..) Done. https://codereview.chromium.org/482163002/diff/250001/chrome/utility/chrome_c... chrome/utility/chrome_content_utility_client.h:81: std::set<int> message_id_whitelist_; On 2014/09/25 02:15:03, Mr4D wrote: > The comment should be a line above and states something like: > > // The maximum IPC message size which can be overridden for testing purposes. Done. https://codereview.chromium.org/482163002/diff/250001/chrome/utility/chrome_c... File chrome/utility/chrome_content_utility_client_unittest.cc (right): https://codereview.chromium.org/482163002/diff/250001/chrome/utility/chrome_c... chrome/utility/chrome_content_utility_client_unittest.cc:44: size_t max_message_size = IPC::Channel::kMaximumMessageSize / 1024; On 2014/09/25 02:15:03, Mr4D wrote: > const size_t ..kTestMessageSize = .. Done.
Message was sent while issue was closed.
lgtm [after fixing the last nit] https://codereview.chromium.org/482163002/diff/270001/chrome/utility/chrome_c... File chrome/utility/chrome_content_utility_client_unittest.cc (right): https://codereview.chromium.org/482163002/diff/270001/chrome/utility/chrome_c... chrome/utility/chrome_content_utility_client_unittest.cc:49: int max_height_for_msg = sqrt(kTestMessageSize/(1.5*4)); sqrt(kTestMessageSize / (1.5 * 4)) (Sorry haven't seen that earlier)
Message was sent while issue was closed.
https://codereview.chromium.org/482163002/diff/270001/chrome/utility/chrome_c... File chrome/utility/chrome_content_utility_client_unittest.cc (right): https://codereview.chromium.org/482163002/diff/270001/chrome/utility/chrome_c... chrome/utility/chrome_content_utility_client_unittest.cc:49: int max_height_for_msg = sqrt(kTestMessageSize/(1.5*4)); On 2014/09/25 19:34:48, Mr4D wrote: > sqrt(kTestMessageSize / (1.5 * 4)) > > (Sorry haven't seen that earlier) Done.
The CQ bit was checked by skuhne@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/482163002/290001
The CQ bit was unchecked by commit-bot@chromium.org
Exceeded time limit waiting for builds to trigger.
The CQ bit was checked by skuhne@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/482163002/290001
Message was sent while issue was closed.
Committed patchset #8 (id:290001) as a68e15738c20bdf6372419270e51462c11ff8341
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/b18f4aa7717e44c208588b56f534e5e6ab7cd69e Cr-Commit-Position: refs/heads/master@{#296939} |