|
|
Chromium Code Reviews|
Created:
4 years ago by Evan Stade Modified:
4 years ago CC:
chromium-reviews, rsesek+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionUpdate WM shadows for MD.
The shadows are still tiled by a ninebox layer, but the images that are
tiled are drawn programatically instead of using assets.
BUG=608852
Committed: https://crrev.com/d1acf8520e37fe497857946d2171989ad3feddd8
Cr-Commit-Position: refs/heads/master@{#437046}
Patch Set 1 #Patch Set 2 : work better #Patch Set 3 : fix corners #
Total comments: 41
Patch Set 4 : jamescook review + resolve merge #Patch Set 5 : fix test #Patch Set 6 : fix test #Patch Set 7 : improve legibility #
Total comments: 8
Patch Set 8 : sky review #Patch Set 9 : fix border for small windows #
Messages
Total messages: 35 (25 generated)
The CQ bit was checked by estade@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== WIP - update WM shadows for MD. The shadows are still tiled by a ninebox layer, but the images that are tiled are drawn programatically instead of using assets. BUG=608852 ========== to ========== Update WM shadows for MD. The shadows are still tiled by a ninebox layer, but the images that are tiled are drawn programatically instead of using assets. BUG=608852 ==========
estade@chromium.org changed reviewers: + jamescook@chromium.org
James, ptal (I will add more relevant owners later if this lgty).
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Overall, I like it. https://codereview.chromium.org/2550593002/diff/40001/ash/common/frame/defaul... File ash/common/frame/default_header_painter.cc (right): https://codereview.chromium.org/2550593002/diff/40001/ash/common/frame/defaul... ash/common/frame/default_header_painter.cc:146: paint.setAntiAlias(true); Why does this now require AA? https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/image/image_skia... File ui/gfx/image/image_skia_operations.cc (right): https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/image/image_skia... ui/gfx/image/image_skia_operations.cc:404: corner_radius_(corner_radius) {} nit: DCHECK something about shadows.size() or .empty() -- it should be non-empty, right? https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/image/image_skia... ui/gfx/image/image_skia_operations.cc:414: gfx::RectF rrect_bounds(bounds); nit: Either rounded_rect_bounds or comment that rrect = rounded rect in skia-land https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/image/image_skia... File ui/gfx/image/image_skia_operations.h (right): https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/image/image_skia... ui/gfx/image/image_skia_operations.h:95: // shadows appropriately with no extra space reserved for "content". nice comment https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/shadow_value.cc File ui/gfx/shadow_value.cc (right): https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/shadow_value.cc#... ui/gfx/shadow_value.cc:29: int blur = nit: this might be clearer as int blur = include_inner_blur ? shadow.blur() + 0.5 : shadow.blur() / 2.0 + 0.5 Optionally dividing by 1 seems a bit weird to me. https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/shadow_value.cc#... ui/gfx/shadow_value.cc:30: static_cast<int>(shadow.blur() / (include_inner_blur ? 1 : 2) + 0.5); optional: Style guide is now OK with int{blah} for numerical type conversions https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc File ui/wm/core/shadow.cc (left): https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ol... ui/wm/core/shadow.cc:156: image = res.GetImageNamed(IDR_AURA_SHADOW_ACTIVE); Can you file a bug about updating Mustash to use the new shadow approach? Code is in ash/mus/shadow.cc. Then we can delete the assets. Label Proj-Mustash-Mash on the bug should be sufficient. https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc File ui/wm/core/shadow.cc (right): https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ne... ui/wm/core/shadow.cc:31: gfx::ImageSkia image; nit: document either here or above struct what this image is (or rename) https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ne... ui/wm/core/shadow.cc:40: g_shadow_cache.Get().insert(std::make_pair(elevation, ShadowDetails())); optional: would emplace work here? https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ne... ui/wm/core/shadow.cc:43: if (insertion.second) { I think this would be clearer if it did std::map::find() or std::map::count() rather than an insert that sometimes fails. If you used find() and you were particularly concerned about performance you could use insert-with-hint. https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ne... ui/wm/core/shadow.cc:63: Shadow::Shadow() : style_(STYLE_ACTIVE) {} optional: init in header? https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ne... ui/wm/core/shadow.cc:154: const gfx::Insets margins = gfx::ShadowValue::GetMargin(details.values); nit: maybe comment that these insets push the bounds outward? (Or do I have that backwards?) https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ne... ui/wm/core/shadow.cc:181: } I'm having a hard time following this function. Maybe a little ascii art picture would help? Or link to a drawing? https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.h File ui/wm/core/shadow.h (right): https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.h#new... ui/wm/core/shadow.h:73: // characteristics. nit: Document that elevation currently means size of shadow in pixels, or at least that shadow size is directly proportional to elevation. I think it's important that elevation isn't just 1,2,3 https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow_unitt... File ui/wm/core/shadow_unittest.cc (right): https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow_unitt... ui/wm/core/shadow_unittest.cc:13: class ShadowTest : public aura::test::AuraTestBase { using ShadowTest = aura::test::AuraTestBase? https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow_unitt... ui/wm/core/shadow_unittest.cc:32: shadow_bounds.Inset(-gfx::Insets(2 * elevation) + Can you comment on what this is expecting/testing? https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow_unitt... ui/wm/core/shadow_unittest.cc:39: shadow_bounds = content_bounds; nit: Either use a new variable, or split the test into two tests. I've been burned by recycling variables in tests many times. :-) https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow_unitt... ui/wm/core/shadow_unittest.cc:46: } // namespace Nice. The linker thanks you.
thanks for the expeditious review! https://codereview.chromium.org/2550593002/diff/40001/ash/common/frame/defaul... File ash/common/frame/default_header_painter.cc (right): https://codereview.chromium.org/2550593002/diff/40001/ash/common/frame/defaul... ash/common/frame/default_header_painter.cc:146: paint.setAntiAlias(true); On 2016/12/05 23:38:35, James Cook wrote: > Why does this now require AA? It should have always had AA, but it wasn't particularly noticeable because there used to be a border stroke superimposed over the corner and that stroke did have AA. Now there's no stroke, just a shadow, so the jaggedness (and asymmetry of left and right corners) looks pretty bad. https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/image/image_skia... File ui/gfx/image/image_skia_operations.cc (right): https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/image/image_skia... ui/gfx/image/image_skia_operations.cc:404: corner_radius_(corner_radius) {} On 2016/12/05 23:38:35, James Cook wrote: > nit: DCHECK something about shadows.size() or .empty() -- it should be > non-empty, right? Done. https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/image/image_skia... ui/gfx/image/image_skia_operations.cc:414: gfx::RectF rrect_bounds(bounds); On 2016/12/05 23:38:35, James Cook wrote: > nit: Either rounded_rect_bounds or comment that rrect = rounded rect in > skia-land removed variable https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/image/image_skia... File ui/gfx/image/image_skia_operations.h (right): https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/image/image_skia... ui/gfx/image/image_skia_operations.h:95: // shadows appropriately with no extra space reserved for "content". On 2016/12/05 23:38:35, James Cook wrote: > nice comment thanks https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/shadow_value.cc File ui/gfx/shadow_value.cc (right): https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/shadow_value.cc#... ui/gfx/shadow_value.cc:29: int blur = On 2016/12/05 23:38:35, James Cook wrote: > nit: this might be clearer as > > int blur = include_inner_blur > ? shadow.blur() + 0.5 > : shadow.blur() / 2.0 + 0.5 > > Optionally dividing by 1 seems a bit weird to me. re-arranged https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/shadow_value.cc#... ui/gfx/shadow_value.cc:30: static_cast<int>(shadow.blur() / (include_inner_blur ? 1 : 2) + 0.5); On 2016/12/05 23:38:35, James Cook wrote: > optional: Style guide is now OK with int{blah} for numerical type conversions Done. https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc File ui/wm/core/shadow.cc (left): https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ol... ui/wm/core/shadow.cc:156: image = res.GetImageNamed(IDR_AURA_SHADOW_ACTIVE); On 2016/12/05 23:38:35, James Cook wrote: > Can you file a bug about updating Mustash to use the new shadow approach? Code > is in ash/mus/shadow.cc. Then we can delete the assets. Label Proj-Mustash-Mash > on the bug should be sufficient. Done. https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc File ui/wm/core/shadow.cc (right): https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ne... ui/wm/core/shadow.cc:31: gfx::ImageSkia image; On 2016/12/05 23:38:35, James Cook wrote: > nit: document either here or above struct what this image is (or rename) Done. https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ne... ui/wm/core/shadow.cc:40: g_shadow_cache.Get().insert(std::make_pair(elevation, ShadowDetails())); On 2016/12/05 23:38:35, James Cook wrote: > optional: would emplace work here? yes it would if map::emplace were allowed :~( "std::map::emplace() is not yet available on all libstdc++ versions we support" - https://chromium-cpp.appspot.com/ https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ne... ui/wm/core/shadow.cc:43: if (insertion.second) { On 2016/12/05 23:38:35, James Cook wrote: > I think this would be clearer if it did std::map::find() or std::map::count() > rather than an insert that sometimes fails. If you used find() and you were > particularly concerned about performance you could use insert-with-hint. Done. https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ne... ui/wm/core/shadow.cc:63: Shadow::Shadow() : style_(STYLE_ACTIVE) {} On 2016/12/05 23:38:35, James Cook wrote: > optional: init in header? Done. https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ne... ui/wm/core/shadow.cc:154: const gfx::Insets margins = gfx::ShadowValue::GetMargin(details.values); On 2016/12/05 23:38:35, James Cook wrote: > nit: maybe comment that these insets push the bounds outward? > > (Or do I have that backwards?) done https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ne... ui/wm/core/shadow.cc:181: } On 2016/12/05 23:38:35, James Cook wrote: > I'm having a hard time following this function. Maybe a little ascii art picture > would help? Or link to a drawing? no kidding. Figuring this out cost me a lot of hair. I think the best docs are in nine_patch_layer.h. I do happen to think that API is pretty darn confusing (especially the fact that the border is a rect instead of Insets), which is probably the reason for the excessive length of the comments in that file. I'm hoping to discuss improvements to it with somebody. I started that conversation on a separate CL already: https://codereview.chromium.org/2543473005/ I created this jsfiddle to help me reason about it (and define what Sebastien really wanted): http://jsfiddle.net/evanstade/40acqxwp/2/ --- do you think that is helpful documentation? I also elaborated some of the text comments in this function. https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.h File ui/wm/core/shadow.h (right): https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.h#new... ui/wm/core/shadow.h:73: // characteristics. On 2016/12/05 23:38:35, James Cook wrote: > nit: Document that elevation currently means size of shadow in pixels, or at > least that shadow size is directly proportional to elevation. I think it's > important that elevation isn't just 1,2,3 Done. https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow_unitt... File ui/wm/core/shadow_unittest.cc (right): https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow_unitt... ui/wm/core/shadow_unittest.cc:13: class ShadowTest : public aura::test::AuraTestBase { On 2016/12/05 23:38:36, James Cook wrote: > using ShadowTest = aura::test::AuraTestBase? Done. https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow_unitt... ui/wm/core/shadow_unittest.cc:32: shadow_bounds.Inset(-gfx::Insets(2 * elevation) + On 2016/12/05 23:38:35, James Cook wrote: > Can you comment on what this is expecting/testing? beyond the comments that already exist, what details would you recommend adding? https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow_unitt... ui/wm/core/shadow_unittest.cc:39: shadow_bounds = content_bounds; On 2016/12/05 23:38:35, James Cook wrote: > nit: Either use a new variable, or split the test into two tests. I've been > burned by recycling variables in tests many times. :-) Done. https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow_unitt... ui/wm/core/shadow_unittest.cc:46: } // namespace On 2016/12/05 23:38:36, James Cook wrote: > Nice. The linker thanks you. it's very welcome
The CQ bit was checked by estade@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
LGTM assuming the test failures aren't a significant problem. https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc File ui/wm/core/shadow.cc (right): https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ne... ui/wm/core/shadow.cc:40: g_shadow_cache.Get().insert(std::make_pair(elevation, ShadowDetails())); On 2016/12/06 00:53:21, Evan Stade wrote: > On 2016/12/05 23:38:35, James Cook wrote: > > optional: would emplace work here? > > yes it would if map::emplace were allowed :~( > > "std::map::emplace() is not yet available on all libstdc++ versions we support" > - https://chromium-cpp.appspot.com/ I learned something new today! https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ne... ui/wm/core/shadow.cc:181: } On 2016/12/06 00:53:21, Evan Stade wrote: > On 2016/12/05 23:38:35, James Cook wrote: > > I'm having a hard time following this function. Maybe a little ascii art > picture > > would help? Or link to a drawing? > > no kidding. Figuring this out cost me a lot of hair. I think the best docs are > in nine_patch_layer.h. I do happen to think that API is pretty darn confusing > (especially the fact that the border is a rect instead of Insets), which is > probably the reason for the excessive length of the comments in that file. I'm > hoping to discuss improvements to it with somebody. I started that conversation > on a separate CL already: https://codereview.chromium.org/2543473005/ > > I created this jsfiddle to help me reason about it (and define what Sebastien > really wanted): http://jsfiddle.net/evanstade/40acqxwp/2/ --- do you think that > is helpful documentation? > > I also elaborated some of the text comments in this function. The new docs here help. I think adding a note about nine_patch_layer.h and a link to your jsfiddle would help too. https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow_unitt... File ui/wm/core/shadow_unittest.cc (right): https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow_unitt... ui/wm/core/shadow_unittest.cc:32: shadow_bounds.Inset(-gfx::Insets(2 * elevation) + On 2016/12/06 00:53:21, Evan Stade wrote: > On 2016/12/05 23:38:35, James Cook wrote: > > Can you comment on what this is expecting/testing? > > beyond the comments that already exist, what details would you recommend adding? Actually, this is fine.
The CQ bit was checked by estade@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
estade@chromium.org changed reviewers: + sky@chromium.org
+sky for OWNERS of everything https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/shadow_value.cc File ui/gfx/shadow_value.cc (right): https://codereview.chromium.org/2550593002/diff/40001/ui/gfx/shadow_value.cc#... ui/gfx/shadow_value.cc:30: static_cast<int>(shadow.blur() / (include_inner_blur ? 1 : 2) + 0.5); On 2016/12/06 00:53:21, Evan Stade wrote: > On 2016/12/05 23:38:35, James Cook wrote: > > optional: Style guide is now OK with int{blah} for numerical type conversions > > Done. Windows didn't like it --- cast from double to int requires a narrowing conversion https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc File ui/wm/core/shadow.cc (right): https://codereview.chromium.org/2550593002/diff/40001/ui/wm/core/shadow.cc#ne... ui/wm/core/shadow.cc:181: } On 2016/12/06 18:17:51, James Cook wrote: > On 2016/12/06 00:53:21, Evan Stade wrote: > > On 2016/12/05 23:38:35, James Cook wrote: > > > I'm having a hard time following this function. Maybe a little ascii art > > picture > > > would help? Or link to a drawing? > > > > no kidding. Figuring this out cost me a lot of hair. I think the best docs are > > in nine_patch_layer.h. I do happen to think that API is pretty darn confusing > > (especially the fact that the border is a rect instead of Insets), which is > > probably the reason for the excessive length of the comments in that file. I'm > > hoping to discuss improvements to it with somebody. I started that > conversation > > on a separate CL already: https://codereview.chromium.org/2543473005/ > > > > I created this jsfiddle to help me reason about it (and define what Sebastien > > really wanted): http://jsfiddle.net/evanstade/40acqxwp/2/ --- do you think > that > > is helpful documentation? > > > > I also elaborated some of the text comments in this function. > > The new docs here help. I think adding a note about nine_patch_layer.h and a > link to your jsfiddle would help too. > done, except included a CSS snippet instead of the fiddle.
The CQ bit was checked by estade@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
LGTM https://codereview.chromium.org/2550593002/diff/120001/ui/gfx/image/image_ski... File ui/gfx/image/image_skia_operations.cc (right): https://codereview.chromium.org/2550593002/diff/120001/ui/gfx/image/image_ski... ui/gfx/image/image_skia_operations.cc:443: float corner_radius_; const https://codereview.chromium.org/2550593002/diff/120001/ui/gfx/shadow_value.cc File ui/gfx/shadow_value.cc (right): https://codereview.chromium.org/2550593002/diff/120001/ui/gfx/shadow_value.cc... ui/gfx/shadow_value.cc:42: } // namespace https://codereview.chromium.org/2550593002/diff/120001/ui/wm/core/shadow.cc File ui/wm/core/shadow.cc (right): https://codereview.chromium.org/2550593002/diff/120001/ui/wm/core/shadow.cc#n... ui/wm/core/shadow.cc:37: typedef std::map<int, ShadowDetails> ShadowDetailsMap; using https://codereview.chromium.org/2550593002/diff/120001/ui/wm/core/shadow.h File ui/wm/core/shadow.h (right): https://codereview.chromium.org/2550593002/diff/120001/ui/wm/core/shadow.h#ne... ui/wm/core/shadow.h:72: // Returns the "elevation" for |style_|, which dictates the shadow's display elevation doesn't really convey what this means, but then I don't have a better suggestion.
The CQ bit was checked by estade@chromium.org to run a CQ dry run
https://codereview.chromium.org/2550593002/diff/120001/ui/gfx/image/image_ski... File ui/gfx/image/image_skia_operations.cc (right): https://codereview.chromium.org/2550593002/diff/120001/ui/gfx/image/image_ski... ui/gfx/image/image_skia_operations.cc:443: float corner_radius_; On 2016/12/07 16:39:32, sky wrote: > const Done. https://codereview.chromium.org/2550593002/diff/120001/ui/gfx/shadow_value.cc File ui/gfx/shadow_value.cc (right): https://codereview.chromium.org/2550593002/diff/120001/ui/gfx/shadow_value.cc... ui/gfx/shadow_value.cc:42: } On 2016/12/07 16:39:32, sky wrote: > // namespace Done. https://codereview.chromium.org/2550593002/diff/120001/ui/wm/core/shadow.cc File ui/wm/core/shadow.cc (right): https://codereview.chromium.org/2550593002/diff/120001/ui/wm/core/shadow.cc#n... ui/wm/core/shadow.cc:37: typedef std::map<int, ShadowDetails> ShadowDetailsMap; On 2016/12/07 16:39:32, sky wrote: > using Done. https://codereview.chromium.org/2550593002/diff/120001/ui/wm/core/shadow.h File ui/wm/core/shadow.h (right): https://codereview.chromium.org/2550593002/diff/120001/ui/wm/core/shadow.h#ne... ui/wm/core/shadow.h:72: // Returns the "elevation" for |style_|, which dictates the shadow's display On 2016/12/07 16:39:32, sky wrote: > elevation doesn't really convey what this means, but then I don't have a better > suggestion. It's the metaphor that MD uses, and it works fairly well in the sense that higher blur values mimic real-world shadows that would be cast on a more distant surface. The visual effect is to make the thing being shadowed appear more elevated in the Z dimension.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by estade@chromium.org
The CQ bit was checked by estade@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jamescook@chromium.org, sky@chromium.org Link to the patchset: https://codereview.chromium.org/2550593002/#ps160001 (title: "fix border for small windows")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 160001, "attempt_start_ts": 1481139780780010,
"parent_rev": "47c580cf983ba200fadcf9a3550d368e1e439f30", "commit_rev":
"54796a8013e202da9bd24f1694ed0a38f2fc9654"}
Message was sent while issue was closed.
Description was changed from ========== Update WM shadows for MD. The shadows are still tiled by a ninebox layer, but the images that are tiled are drawn programatically instead of using assets. BUG=608852 ========== to ========== Update WM shadows for MD. The shadows are still tiled by a ninebox layer, but the images that are tiled are drawn programatically instead of using assets. BUG=608852 ==========
Message was sent while issue was closed.
Committed patchset #9 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== Update WM shadows for MD. The shadows are still tiled by a ninebox layer, but the images that are tiled are drawn programatically instead of using assets. BUG=608852 ========== to ========== Update WM shadows for MD. The shadows are still tiled by a ninebox layer, but the images that are tiled are drawn programatically instead of using assets. BUG=608852 Committed: https://crrev.com/d1acf8520e37fe497857946d2171989ad3feddd8 Cr-Commit-Position: refs/heads/master@{#437046} ==========
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/d1acf8520e37fe497857946d2171989ad3feddd8 Cr-Commit-Position: refs/heads/master@{#437046} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
