Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1408063012:
Replaced GetAppIconForSize with GetAppIconImageFamily. (Closed)
|
Created:
5 years, 1 month ago by Matt Giuca Modified:
5 years, 1 month ago CC:
chromium-reviews, tfarina, rsesek+watch_chromium.org, chrome-apps-syd-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master-plus Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionReplaced GetAppIconForSize with GetAppIconImageFamily.
The old API gets a single icon from a Windows ICO of a desired size, but
it would use a Windows API that automatically scales the icon using a
hideous scaling algorithm. The new API loads all images of the icon into
an ImageFamily which can be used with CreateExact to perform a
high-quality scaling if necessary.
Updated ProfileShortcutManagerWin to use GetAppIconImageFamily.
Currently should have no visible effects, but will enable us to replace
some other code and use much higher quality scaling (specifically, the
system tray icon; see crbug.com/124141).
BUG=551256
Committed: https://crrev.com/3e9e69dc9fbd729e77c66bfbabf4c7416ee2ecf8
Cr-Commit-Position: refs/heads/master@{#360256}
Patch Set 1 #Patch Set 2 : Split off ImageFamily stuff into CL 1424913007. #
Total comments: 2
Patch Set 3 : Rebase. #Patch Set 4 : Return null scoped_ptr instead of empty SkBitmap. #
Total comments: 4
Patch Set 5 : Deleted GetAppIconForSize. #Patch Set 6 : Fix IconUtilTest. #Patch Set 7 : Fix unit_tests by loading from the current module if chrome.dll is not loaded. #
Total comments: 2
Messages
Total messages: 47 (19 generated)
mgiuca@chromium.org changed reviewers: + calamity@chromium.org
calamity: What do you think of this approach? I will probably split the ImageFamily changes into a separate CL so you could wait until I do that before having a look at this (if it's too hard to read at the moment).
Description was changed from ========== GetAppIconForSize now performs nice high-quality scaling. Currently should have no visible effects, but will enable us to replace some other code and use much higher quality scaling (specifically, the system tray icon; see crbug.com/124141). Previously, it used the Windows API to get an icon of the desired size, which would automatically scale the icon using a hideous scaling algorithm. Now loads the icon into an ImageFamily and then performs a high-quality scaling algorithm if necessary. Updated a number of other internal APIs: - Adds GetAppIconImageFamily (so you can get all sizes at once). - Adds ImageFamily::CreateExact, which does high-quality scaling. Updated clients that were doing manual scaling to use CreateExact. BUG=551256 ========== to ========== GetAppIconForSize now performs nice high-quality scaling. Currently should have no visible effects, but will enable us to replace some other code and use much higher quality scaling (specifically, the system tray icon; see crbug.com/124141). Previously, it used the Windows API to get an icon of the desired size, which would automatically scale the icon using a hideous scaling algorithm. Now loads the icon into an ImageFamily and then performs a high-quality scaling algorithm if necessary. Updated a number of other internal APIs: - Adds GetAppIconImageFamily (so you can get all sizes at once). BUG=551256 ==========
Patchset #3 (id:40001) has been deleted
On 2015/11/04 07:39:30, Matt Giuca wrote: > calamity: What do you think of this approach? > > I will probably split the ImageFamily changes into a separate CL so you could > wait until I do that before having a look at this (if it's too hard to read at > the moment). PTAL (I've split off the ImageFamily changes). WDYT? The downside of this is that we lose the ability to load an individual icon image of a particular size (without loading all images into memory). An alternative approach involves a rather major refactor of ImageFamily such that you can get all the sizes and run the GetBest algorithm without explicitly decoding all of the image data, then just loading one image. The extra complexity doesn't seem worth it to me.
Description was changed from ========== GetAppIconForSize now performs nice high-quality scaling. Currently should have no visible effects, but will enable us to replace some other code and use much higher quality scaling (specifically, the system tray icon; see crbug.com/124141). Previously, it used the Windows API to get an icon of the desired size, which would automatically scale the icon using a hideous scaling algorithm. Now loads the icon into an ImageFamily and then performs a high-quality scaling algorithm if necessary. Updated a number of other internal APIs: - Adds GetAppIconImageFamily (so you can get all sizes at once). BUG=551256 ========== to ========== GetAppIconForSize now performs nice high-quality scaling. Currently should have no visible effects, but will enable us to replace some other code and use much higher quality scaling (specifically, the system tray icon; see crbug.com/124141). Previously, it used the Windows API to get an icon of the desired size, which would automatically scale the icon using a hideous scaling algorithm. Now loads the icon into an ImageFamily and then performs a high-quality scaling algorithm if necessary. Also adds a number of new functions to app_icon_win: GetAppIconSize, GetAppIconSmallSize and GetAppIconImageFamily. BUG=551256 ==========
Description was changed from ========== GetAppIconForSize now performs nice high-quality scaling. Currently should have no visible effects, but will enable us to replace some other code and use much higher quality scaling (specifically, the system tray icon; see crbug.com/124141). Previously, it used the Windows API to get an icon of the desired size, which would automatically scale the icon using a hideous scaling algorithm. Now loads the icon into an ImageFamily and then performs a high-quality scaling algorithm if necessary. Also adds a number of new functions to app_icon_win: GetAppIconSize, GetAppIconSmallSize and GetAppIconImageFamily. BUG=551256 ========== to ========== GetAppIconForSize now performs nice high-quality scaling. Currently should have no visible effects, but will enable us to replace some other code and use much higher quality scaling (specifically, the system tray icon; see crbug.com/124141). Previously, it used the Windows API to get an icon of the desired size, which would automatically scale the icon using a hideous scaling algorithm. Now loads the icon into an ImageFamily and then performs a high-quality scaling algorithm if necessary. Also adds a number of new functions to app_icon_win: GetAppIconSize, GetSmallAppIconSize and GetAppIconImageFamily. BUG=551256 ==========
lgtm. As discussed, this code is only run during profile shortcut creation and on startup. It would be nice to document the startup time difference. https://codereview.chromium.org/1408063012/diff/20001/chrome/browser/app_icon... File chrome/browser/app_icon_win.cc (right): https://codereview.chromium.org/1408063012/diff/20001/chrome/browser/app_icon... chrome/browser/app_icon_win.cc:70: return make_scoped_ptr(new SkBitmap); nit: Why not just return an empty scoped_ptr?
On 2015/11/09 02:12:11, calamity wrote: > lgtm. > > As discussed, this code is only run during profile shortcut creation and on > startup. It would be nice to document the startup time difference. Note: This doesn't yet run at startup but it will be used for loading the systray icon in a future CL (https://codereview.chromium.org/1420163003/) so we should consider this startup-critical code. I did some simple benchmarks (just measure TimeTicks::Now delta over GetStatusTrayIcon in the next CL, with 3 runs): Timing experiments (system-tray-fromimagefamily) old (use hard-coded 22x22 image): GetStatusTrayIcon time elapsed: 0.00023s GetStatusTrayIcon time elapsed: 0.000228s GetStatusTrayIcon time elapsed: 0.000218s new (only 16x16 icon loaded -- baseline for correct behaviour): GetStatusTrayIcon time elapsed: 0.003479s GetStatusTrayIcon time elapsed: 0.004063s GetStatusTrayIcon time elapsed: 0.003577s new (PNG decoder removed): GetStatusTrayIcon time elapsed: 0.005947s GetStatusTrayIcon time elapsed: 0.005482s GetStatusTrayIcon time elapsed: 0.00651s new (full ImageFamily loaded): GetStatusTrayIcon time elapsed: 0.013856s GetStatusTrayIcon time elapsed: 0.013724s GetStatusTrayIcon time elapsed: 0.01336s What this means is that in order to get the correct (non-hideous) systray icon, we need to go from 0.2ms to 3.7ms at the minimum (to load just the right icon from the ICO file). That assumes a big (and rather ugly) refactor. If we avoid the refactor and just insert a hack to avoid loading the large-size PNG icon, it takes 6.0ms. And the current behaviour for this CL (including loading the PNG) takes 13.6ms. We probably should do one of these things: either hack to avoid loading the PNG, or a full refactor to compute the size before loading the image (essentially avoid using the ImageFamily abstraction). WDYT? > > https://codereview.chromium.org/1408063012/diff/20001/chrome/browser/app_icon... > File chrome/browser/app_icon_win.cc (right): > > https://codereview.chromium.org/1408063012/diff/20001/chrome/browser/app_icon... > chrome/browser/app_icon_win.cc:70: return make_scoped_ptr(new SkBitmap); > nit: Why not just return an empty scoped_ptr?
Crap, those were all from a Debug build. Here are the Release build figures: old (use hard-coded 22x22 image): GetStatusTrayIcon time elapsed: 6.2e-005s GetStatusTrayIcon time elapsed: 5.9e-005s GetStatusTrayIcon time elapsed: 6.4e-005s new (only 16x16 icon loaded -- baseline for correct behaviour): GetStatusTrayIcon time elapsed: 0.003625s GetStatusTrayIcon time elapsed: 0.00346s GetStatusTrayIcon time elapsed: 0.003639s new (PNG decoder removed): GetStatusTrayIcon time elapsed: 0.005372s GetStatusTrayIcon time elapsed: 0.005272s GetStatusTrayIcon time elapsed: 0.005099s new (full ImageFamily loaded): GetStatusTrayIcon time elapsed: 0.007064s GetStatusTrayIcon time elapsed: 0.006796s GetStatusTrayIcon time elapsed: 0.006619s OK so only half as bad in the ImageFamily case. Maybe this is OK now. To put it in context, my overall startup time (Startup.FirstWebContents.NonEmptyPaint) is 1.13s, warm start. So this would be increasing startup time by 0.6%, for users with background mode enabled.
OK, it turns out it's quite easy to post this as an after-startup task. However, that severely delays showing the system tray icon (up to about 5 seconds). I'm happy to go ahead with this CL (which, in itself, does not impact startup time). In the subsequent CL (https://codereview.chromium.org/1420163003/) we can decide whether to a) slightly increase startup time, or b) post it as a task.
https://codereview.chromium.org/1408063012/diff/20001/chrome/browser/app_icon... File chrome/browser/app_icon_win.cc (right): https://codereview.chromium.org/1408063012/diff/20001/chrome/browser/app_icon... chrome/browser/app_icon_win.cc:70: return make_scoped_ptr(new SkBitmap); On 2015/11/09 02:12:11, calamity wrote: > nit: Why not just return an empty scoped_ptr? Done.
Patchset #3 (id:60001) has been deleted
mgiuca@chromium.org changed reviewers: + danakj@chromium.org, thakis@chromium.org
Hi Nico, Dana: PTAL. This CL is a bit weird because it refactors GetAppIconForSize and also removes its only caller (because that is using a more efficient function now). But GAIFS will be used in the follow-up CL (https://codereview.chromium.org/1420163003/) so I don't want to delete it. Please note the above discussion about performance but note that it will not affect startup performance in this CL; that is a matter for the next CL.
lgtm with a few nit-like comments, the first one is maybe the most interesting https://codereview.chromium.org/1408063012/diff/100001/chrome/browser/app_ico... File chrome/browser/app_icon_win.cc (right): https://codereview.chromium.org/1408063012/diff/100001/chrome/browser/app_ico... chrome/browser/app_icon_win.cc:75: gfx::Image image = family->CreateExact(size); gfx::Image can contain several versions of the same bitmap at different scaling factors. Do we use this on Windows? (On Mac, this is used to let the OS pick the right version of a bitmap when you drag a chrome window from a retina screen to a regular screen and back.) Since you only care about one bitmap in the image here, you're doing more computation here than needed. https://codereview.chromium.org/1408063012/diff/100001/chrome/browser/app_ico... File chrome/browser/app_icon_win.h (right): https://codereview.chromium.org/1408063012/diff/100001/chrome/browser/app_ico... chrome/browser/app_icon_win.h:33: // this repeatedly as it internally builds the ImageFamily each time. Is it possible to remove these functions here then? Working with the ImageFamily shouldn't be much harder, and not everyone adding a call to these will read this comment.
Patchset #5 (id:120001) has been deleted
Description was changed from ========== Replaced GetAppIconForSize with GetAppIconImageFamily. The old API gets a single icon from a Windows ICO of a desired size, but it would use a Windows API that automatically scales the icon using a hideous scaling algorithm. The new API loads all images of the icon into an ImageFamily which can be used with CreateExact to perform a high-quality scaling if necessary. Updated ProfileShortcutManagerWin to use GetAppIconImageFamily. Currently should have no visible effects, but will enable us to replace some other code and use much higher quality scaling (specifically, the system tray icon; see crbug.com/124141). BUG=551256 ========== to ========== Replaced GetAppIconForSize with GetAppIconImageFamily. The old API gets a single icon from a Windows ICO of a desired size, but it would use a Windows API that automatically scales the icon using a hideous scaling algorithm. The new API loads all images of the icon into an ImageFamily which can be used with CreateExact to perform a high-quality scaling if necessary. Updated ProfileShortcutManagerWin to use GetAppIconImageFamily. Currently should have no visible effects, but will enable us to replace some other code and use much higher quality scaling (specifically, the system tray icon; see crbug.com/124141). BUG=551256 ==========
Removing GetAppIconForSize kind of changes the whole focus of the CL, so rewrote the CL description. https://codereview.chromium.org/1408063012/diff/100001/chrome/browser/app_ico... File chrome/browser/app_icon_win.cc (right): https://codereview.chromium.org/1408063012/diff/100001/chrome/browser/app_ico... chrome/browser/app_icon_win.cc:75: gfx::Image image = family->CreateExact(size); On 2015/11/10 07:13:31, Nico wrote: > gfx::Image can contain several versions of the same bitmap at different scaling > factors. Do we use this on Windows? (On Mac, this is used to let the OS pick the > right version of a bitmap when you drag a chrome window from a retina screen to > a regular screen and back.) Short answer: no, we don't use gfx::Image scale factors because we don't know what density Windows wants, only what size it wants. (Therefore, ImageFamily is appropriate.) In this case, we are not dealing with low/high DPI versions of the same logical image. We are dealing with multiple images of different logical sizes. (A 32x32 icon image is NOT a high-DPI version of a 16x16 icon image; it is a larger image.) This is the reason I created ImageFamily in the first place: to capture the distinction between low/high DPI versus small/large logical size. (At the time, we had code putting icon images of all sizes into an ImageSkia as different "densities" which was a hack.) So the nice theory (which I think is fairly clear on Mac) is that you have a "logical size" (e.g., 16x16) and then a "density" (e.g., 150%), in which case you'd pick the 16x16 Image in the ImageFamily an then get its 150% representation from the ImageSkia or NSImage. Unfortunately, Windows blurs the concept of size and density, because at least for these GetSystemMetrics APIs, we are just given a size. If the user's display is set to 150%, GetSystemMetrics(SM_CXSMICON) just returns 24. So all we can really do is put it in an ImageFamily, and then take the best sized icon and scale it to 24. > Since you only care about one bitmap in the image > here, you're doing more computation here than needed. Yeah, I am. This is the price I'm paying for use of the nice ImageFamily abstraction. The logic (using this abstraction) is: 1. Parse the ICO header table to find out what sizes are available. 2. Load all the images into an ImageFamily. 3. Choose the best one for the target size. 4. Scale it to the target size. Without ImageFamily, we'd have to do this: 1. Parse the ICO header table to find out what sizes are available. 2. Choose the best one for the target size. 3. Load that specific image. 4. Scale it to the target size. The alternative approach avoids loading unnecessary images, but it means duplicating or significantly refactoring most of the logic in ImageFamily. I've thought a lot about how to do that, and I did benchmarks above which showed that by only loading the image we need, we could reduce a 6ms operation to 3ms. I don't think that's worth the added complexity in the code, given that this will only be used once on startup, in a delayed task. https://codereview.chromium.org/1408063012/diff/100001/chrome/browser/app_ico... File chrome/browser/app_icon_win.h (right): https://codereview.chromium.org/1408063012/diff/100001/chrome/browser/app_ico... chrome/browser/app_icon_win.h:33: // this repeatedly as it internally builds the ImageFamily each time. On 2015/11/10 07:13:31, Nico wrote: > Is it possible to remove these functions here then? Working with the ImageFamily > shouldn't be much harder, and not everyone adding a call to these will read this > comment. Fair enough. I will just inline this code when I need it. Done.
On 2015/11/11 02:36:23, Matt Giuca wrote: > Removing GetAppIconForSize kind of changes the whole focus of the CL, so rewrote > the CL description. > > https://codereview.chromium.org/1408063012/diff/100001/chrome/browser/app_ico... > File chrome/browser/app_icon_win.cc (right): > > https://codereview.chromium.org/1408063012/diff/100001/chrome/browser/app_ico... > chrome/browser/app_icon_win.cc:75: gfx::Image image = family->CreateExact(size); > On 2015/11/10 07:13:31, Nico wrote: > > gfx::Image can contain several versions of the same bitmap at different > scaling > > factors. Do we use this on Windows? (On Mac, this is used to let the OS pick > the > > right version of a bitmap when you drag a chrome window from a retina screen > to > > a regular screen and back.) > > Short answer: no, we don't use gfx::Image scale factors because we don't know > what density Windows wants, only what size it wants. (Therefore, ImageFamily is > appropriate.) > > In this case, we are not dealing with low/high DPI versions of the same logical > image. We are dealing with multiple images of different logical sizes. (A 32x32 > icon image is NOT a high-DPI version of a 16x16 icon image; it is a larger > image.) This is the reason I created ImageFamily in the first place: to capture > the distinction between low/high DPI versus small/large logical size. (At the > time, we had code putting icon images of all sizes into an ImageSkia as > different "densities" which was a hack.) > > So the nice theory (which I think is fairly clear on Mac) is that you have a > "logical size" (e.g., 16x16) and then a "density" (e.g., 150%), in which case > you'd pick the 16x16 Image in the ImageFamily an then get its 150% > representation from the ImageSkia or NSImage. Unfortunately, Windows blurs the > concept of size and density, because at least for these GetSystemMetrics APIs, > we are just given a size. If the user's display is set to 150%, > GetSystemMetrics(SM_CXSMICON) just returns 24. > > So all we can really do is put it in an ImageFamily, and then take the best > sized icon and scale it to 24. > > > Since you only care about one bitmap in the image > > here, you're doing more computation here than needed. > > Yeah, I am. This is the price I'm paying for use of the nice ImageFamily > abstraction. The logic (using this abstraction) is: > 1. Parse the ICO header table to find out what sizes are available. > 2. Load all the images into an ImageFamily. > 3. Choose the best one for the target size. > 4. Scale it to the target size. > > Without ImageFamily, we'd have to do this: > 1. Parse the ICO header table to find out what sizes are available. > 2. Choose the best one for the target size. > 3. Load that specific image. > 4. Scale it to the target size. > > The alternative approach avoids loading unnecessary images, but it means > duplicating or significantly refactoring most of the logic in ImageFamily. I've > thought a lot about how to do that, and I did benchmarks above which showed that > by only loading the image we need, we could reduce a 6ms operation to 3ms. I > don't think that's worth the added complexity in the code, given that this will > only be used once on startup, in a delayed task. > > https://codereview.chromium.org/1408063012/diff/100001/chrome/browser/app_ico... > File chrome/browser/app_icon_win.h (right): > > https://codereview.chromium.org/1408063012/diff/100001/chrome/browser/app_ico... > chrome/browser/app_icon_win.h:33: // this repeatedly as it internally builds the > ImageFamily each time. > On 2015/11/10 07:13:31, Nico wrote: > > Is it possible to remove these functions here then? Working with the > ImageFamily > > shouldn't be much harder, and not everyone adding a call to these will read > this > > comment. > > Fair enough. I will just inline this code when I need it. > > Done. OK since I have lgtms I think I will land this now. (I vaguely feel that I should wait for Nico and Dana final approval, but since no reply since yesterday, I'll assume this is good to go.)
The CQ bit was checked by mgiuca@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from calamity@chromium.org, thakis@chromium.org Link to the patchset: https://codereview.chromium.org/1408063012/#ps140001 (title: "Deleted GetAppIconForSize.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1408063012/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1408063012/140001
yeah, still lgtm sorry about the latency, both Dana and I were at blinkon
On 2015/11/12 04:30:53, Nico wrote: > yeah, still lgtm > > sorry about the latency, both Dana and I were at blinkon That's OK, I just wasn't sure if I was supposed to wait for another round (but figured that if it's all green, may as well go for it :)).
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...) win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
Blerg, I had to update some tests which I forgot to do before (when the API changed). Hope this is OK; going to try landing again.
The CQ bit was checked by mgiuca@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from calamity@chromium.org, thakis@chromium.org Link to the patchset: https://codereview.chromium.org/1408063012/#ps160001 (title: "Fix IconUtilTest.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1408063012/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1408063012/160001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
On 2015/11/12 04:30:53, Nico wrote: > yeah, still lgtm > > sorry about the latency, both Dana and I were at blinkon ditto. i'm happy if nico is happy.
The CQ bit was checked by mgiuca@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1408063012/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1408063012/180001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Hi Nico, Sorry to do this but would you be able to review the delta between PS 5 and 7? I broke some tests and had to fix them up after your LG. Dry run has passed now. (PTAL)
https://codereview.chromium.org/1408063012/diff/180001/chrome/browser/app_ico... File chrome/browser/app_icon_win.cc (right): https://codereview.chromium.org/1408063012/diff/180001/chrome/browser/app_ico... chrome/browser/app_icon_win.cc:66: // unit_tests.exe, that has the same resource IDs as chrome.dll). why wasn't this needed previously? or am i just missing where this is from?
https://codereview.chromium.org/1408063012/diff/180001/chrome/browser/app_ico... File chrome/browser/app_icon_win.cc (right): https://codereview.chromium.org/1408063012/diff/180001/chrome/browser/app_ico... chrome/browser/app_icon_win.cc:66: // unit_tests.exe, that has the same resource IDs as chrome.dll). On 2015/11/16 19:04:28, Nico wrote: > why wasn't this needed previously? or am i just missing where this is from? That's a great question. It took me some time to figure this one out. The answer is "it just happened to work before" :/ This would call GetModuleHandle(kBrowserResourcesDll) which returned nullptr (in tests). CreateSkBitmapFromIconResource passes this nullptr Win32's LoadImage which, according to its docs "load[s] an OEM image" [1]. I'm not sure what that is but it returns a valid 48x48 icon. With my new code, it goes through base::win::GetResourceFromModule which explicitly checks for a null module. So yeah, this new code here is correct in explicitly checking for null. [1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms648045(v=vs.85).aspx
lgtm
The CQ bit was checked by mgiuca@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from calamity@chromium.org Link to the patchset: https://codereview.chromium.org/1408063012/#ps180001 (title: "Fix unit_tests by loading from the current module if chrome.dll is not loaded.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1408063012/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1408063012/180001
Message was sent while issue was closed.
Committed patchset #7 (id:180001)
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/3e9e69dc9fbd729e77c66bfbabf4c7416ee2ecf8 Cr-Commit-Position: refs/heads/master@{#360256} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
