Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
754763005:
Speculative fix for sticky keys overlay crash. (Closed)
DescriptionSpeculative fix for sticky keys overlay crash.
The new test case reproduces the same stack trace as in the bug, so it's very
probable that this case is causing the crash.
BUG=435600
Committed: https://crrev.com/9391c0e45bc3ae50008d0aebf11437550e7f38c6
Cr-Commit-Position: refs/heads/master@{#306504}
Committed: https://crrev.com/f98986c03fb7047364b9e8bd8c3f74622a315802
Cr-Commit-Position: refs/heads/master@{#307140}
Patch Set 1 #
Total comments: 7
Patch Set 2 : fixes #Patch Set 3 : found repro for this crash #Patch Set 4 : #
Total comments: 6
Patch Set 5 : add more asserts #
Total comments: 16
Patch Set 6 : remove WidgetDelegateView inheritance #
Total comments: 2
Messages
Total messages: 35 (8 generated)
tengs@chromium.org changed reviewers: + jamescook@chromium.org
LGTM with nits. Be sure to run ASAN or Valgrind builds before landing to provide some assurance you aren't leaking. https://codereview.chromium.org/754763005/diff/1/ash/sticky_keys/sticky_keys_... File ash/sticky_keys/sticky_keys_overlay.h (right): https://codereview.chromium.org/754763005/diff/1/ash/sticky_keys/sticky_keys_... ash/sticky_keys/sticky_keys_overlay.h:54: // Returns the underlying ::views Widget for testing purposes. The returned nit: views::Widget https://codereview.chromium.org/754763005/diff/1/ash/sticky_keys/sticky_keys_... ash/sticky_keys/sticky_keys_overlay.h:55: // widget is owned by StickyKeysOverlay. Thanks for an ownership comment! https://codereview.chromium.org/754763005/diff/1/ash/sticky_keys/sticky_keys_... File ash/sticky_keys/sticky_keys_overlay_unittest.cc (right): https://codereview.chromium.org/754763005/diff/1/ash/sticky_keys/sticky_keys_... ash/sticky_keys/sticky_keys_overlay_unittest.cc:15: class StickyKeysOverlayTest : public test::AshTestBase { optional: This could become: using StickyKeysOverlayTest = test::AshTestBase; and no functions. https://codereview.chromium.org/754763005/diff/1/ash/sticky_keys/sticky_keys_... ash/sticky_keys/sticky_keys_overlay_unittest.cc:45: TEST_F(StickyKeysOverlayTest, OverlayViewOwnership) { nit: Document what this test is testing. I like to include a link to the crbug for things testing a crash or specific issue.
https://codereview.chromium.org/754763005/diff/1/ash/sticky_keys/sticky_keys_... File ash/sticky_keys/sticky_keys_overlay.h (right): https://codereview.chromium.org/754763005/diff/1/ash/sticky_keys/sticky_keys_... ash/sticky_keys/sticky_keys_overlay.h:54: // Returns the underlying ::views Widget for testing purposes. The returned On 2014/12/01 21:06:00, James Cook wrote: > nit: views::Widget Done. https://codereview.chromium.org/754763005/diff/1/ash/sticky_keys/sticky_keys_... File ash/sticky_keys/sticky_keys_overlay_unittest.cc (right): https://codereview.chromium.org/754763005/diff/1/ash/sticky_keys/sticky_keys_... ash/sticky_keys/sticky_keys_overlay_unittest.cc:15: class StickyKeysOverlayTest : public test::AshTestBase { On 2014/12/01 21:06:00, James Cook wrote: > optional: This could become: > > using StickyKeysOverlayTest = test::AshTestBase; > > and no functions. Done. https://codereview.chromium.org/754763005/diff/1/ash/sticky_keys/sticky_keys_... ash/sticky_keys/sticky_keys_overlay_unittest.cc:45: TEST_F(StickyKeysOverlayTest, OverlayViewOwnership) { On 2014/12/01 21:06:00, James Cook wrote: > nit: Document what this test is testing. I like to include a link to the crbug > for things testing a crash or specific issue. Done.
The CQ bit was checked by tengs@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/754763005/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel on tryserver.chromium.win (http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
The CQ bit was checked by tengs@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/754763005/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/9391c0e45bc3ae50008d0aebf11437550e7f38c6 Cr-Commit-Position: refs/heads/master@{#306504}
Message was sent while issue was closed.
thestig@chromium.org changed reviewers: + thestig@chromium.org
Message was sent while issue was closed.
Valgrind bots are red, and so are the CrOS ASAN/LSAN bots -> reverting.
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/757433005/ by thestig@chromium.org. The reason for reverting is: StickyKeysBrowserTest leaking memory..
Message was sent while issue was closed.
On 2014/12/03 02:15:32, Lei Zhang wrote: > A revert of this CL (patchset #2 id:20001) has been created in > https://codereview.chromium.org/757433005/ by mailto:thestig@chromium.org. > > The reason for reverting is: StickyKeysBrowserTest leaking memory.. Tim, just FYI the CrOS Valgrind and ASAN bots are not part of the CQ set (they are too slow) and you have to explicitly request running them.
Message was sent while issue was closed.
My apologies. I misunderstood the ownership semantics of WidgetDelegateView yet again. However, I managed to find a repro case for this crash after some digging. You can cause this crash by disconnecting a secondary monitor that is positioned to the left of your primary one. Please take another look. The root window destruction when removing a display is a bit scary and may affect other widgets that do not get reparented.
Message was sent while issue was closed.
A suggestion. Also, if you need input on the specifics of display connect/disconnect then oshima or mukai would be better reviewers. I'm comfortable reviewing this if you'd like to continue with me, however. https://codereview.chromium.org/754763005/diff/60001/ash/sticky_keys/sticky_k... File ash/sticky_keys/sticky_keys_overlay_unittest.cc (right): https://codereview.chromium.org/754763005/diff/60001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay_unittest.cc:45: UpdateDisplay("1280x1024, 1980x1080"); super nit: No space after , https://codereview.chromium.org/754763005/diff/60001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay_unittest.cc:50: // The overlay should belong to the secondary root window. Is there an assert or expect you could make here? https://codereview.chromium.org/754763005/diff/60001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay_unittest.cc:59: EXPECT_EQ(STICKY_KEY_STATE_ENABLED, Likewise I don't see how these assertions really test the reparenting. If your sole goal with this test is "make sure it doesn't crash" then your comments should make that explicit. Or you should have an EXPECT line that validates the reparenting.
tengs@chromium.org changed reviewers: + mukai@chromium.org
+ mukai@ for an additional pair of eyes, in case I'm overlooking something with the display logic. https://codereview.chromium.org/754763005/diff/60001/ash/sticky_keys/sticky_k... File ash/sticky_keys/sticky_keys_overlay_unittest.cc (right): https://codereview.chromium.org/754763005/diff/60001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay_unittest.cc:45: UpdateDisplay("1280x1024, 1980x1080"); On 2014/12/04 20:43:17, James Cook wrote: > super nit: No space after , Done. https://codereview.chromium.org/754763005/diff/60001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay_unittest.cc:50: // The overlay should belong to the secondary root window. On 2014/12/04 20:43:17, James Cook wrote: > Is there an assert or expect you could make here? Done. I added some asserts to make sure the widget is in the expected root window. https://codereview.chromium.org/754763005/diff/60001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay_unittest.cc:59: EXPECT_EQ(STICKY_KEY_STATE_ENABLED, On 2014/12/04 20:43:17, James Cook wrote: > Likewise I don't see how these assertions really test the reparenting. If your > sole goal with this test is "make sure it doesn't crash" then your comments > should make that explicit. Or you should have an EXPECT line that validates the > reparenting. Done.
https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... File ash/sticky_keys/sticky_keys_overlay.cc (right): https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay.cc:225: params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET; WIDGET_OWNS_NATIVE_WIDGET is not used usually. I believe the standard way is: - the default ownership - this class observes the widget, and clears the field like overlay_widget_ at OnWidgetDestroying() https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay.cc:236: overlay_widget_->SetContentsView(overlay_view_.get()); Because overlay_view_ is a WidgetDelegateView, it should inherit GetContentsView() to return itself instead of setting here. https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... File ash/sticky_keys/sticky_keys_overlay.h (right): https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay.h:70: scoped_ptr<StickyKeysOverlayView> overlay_view_; Why it must outlive?
LGTM with nits. Again, be sure to run the ASAN / valgrind bots before landing. https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... File ash/sticky_keys/sticky_keys_overlay.cc (right): https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay.cc:135: set_owned_by_client(); Doesn't the WidgetDelegateView constructor do this for you? https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... File ash/sticky_keys/sticky_keys_overlay_unittest.cc (right): https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay_unittest.cc:50: int64 primary_display_id = display_ids.first; nit: int64_t and next line too https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay_unittest.cc:75: } Much better test!
https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... File ash/sticky_keys/sticky_keys_overlay.cc (right): https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay.cc:135: set_owned_by_client(); On 2014/12/04 22:00:45, James Cook wrote: > Doesn't the WidgetDelegateView constructor do this for you? I just realized that there is no reason to inherit from WidgetDelegateView anymore, so this class will inherit directly from views::View. https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay.cc:225: params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET; On 2014/12/04 21:58:54, Jun Mukai wrote: > WIDGET_OWNS_NATIVE_WIDGET is not used usually. I believe the standard way is: > - the default ownership > - this class observes the widget, and clears the field like overlay_widget_ at > OnWidgetDestroying() I'm using WIDGET_OWNS_NATIVE_WIDGET primarily so if you turn off sticky keys, we can clean up all the resources the feature was using. It seems more difficult to do this without owning the widget. https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay.cc:236: overlay_widget_->SetContentsView(overlay_view_.get()); On 2014/12/04 21:58:54, Jun Mukai wrote: > Because overlay_view_ is a WidgetDelegateView, it should inherit > GetContentsView() to return itself instead of setting here. Done. I stopped inheriting from WidgetDelegateView and inherit from View directly. https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... File ash/sticky_keys/sticky_keys_overlay.h (right): https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay.h:70: scoped_ptr<StickyKeysOverlayView> overlay_view_; On 2014/12/04 21:58:55, Jun Mukai wrote: > Why it must outlive? Done. This is no longer needed after getting rid of the WidgetDelegate. https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... File ash/sticky_keys/sticky_keys_overlay_unittest.cc (right): https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay_unittest.cc:50: int64 primary_display_id = display_ids.first; On 2014/12/04 22:00:45, James Cook wrote: > nit: int64_t and next line too Done. https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay_unittest.cc:75: } On 2014/12/04 22:00:45, James Cook wrote: > Much better test! Thanks!
thestig@chromium.org changed reviewers: - thestig@chromium.org
https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... File ash/sticky_keys/sticky_keys_overlay.cc (right): https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay.cc:225: params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET; On 2014/12/04 23:53:37, Tim Song wrote: > On 2014/12/04 21:58:54, Jun Mukai wrote: > > WIDGET_OWNS_NATIVE_WIDGET is not used usually. I believe the standard way is: > > - the default ownership > > - this class observes the widget, and clears the field like overlay_widget_ at > > OnWidgetDestroying() > > I'm using WIDGET_OWNS_NATIVE_WIDGET primarily so if you turn off sticky keys, we > can clean up all the resources the feature was using. It seems more difficult to > do this without owning the widget. I am still not so sure what's the problem. That sounds like that's because StickyKeysOverlay owns the widget as scoped_ptr, but I am suggesting it (and overlay_view_) don't have to be. I feel they can be raw pointers, and StickyKeysOverlay's dtor can just call overlay_widget_->Close() which cleans up all the resources used for that widget. Is there any specific reason to avoid that?
https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... File ash/sticky_keys/sticky_keys_overlay.cc (right): https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay.cc:225: params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET; On 2014/12/05 00:10:36, Jun Mukai wrote: > On 2014/12/04 23:53:37, Tim Song wrote: > > On 2014/12/04 21:58:54, Jun Mukai wrote: > > > WIDGET_OWNS_NATIVE_WIDGET is not used usually. I believe the standard way > is: > > > - the default ownership > > > - this class observes the widget, and clears the field like overlay_widget_ > at > > > OnWidgetDestroying() > > > > I'm using WIDGET_OWNS_NATIVE_WIDGET primarily so if you turn off sticky keys, > we > > can clean up all the resources the feature was using. It seems more difficult > to > > do this without owning the widget. > > I am still not so sure what's the problem. > That sounds like that's because StickyKeysOverlay owns the widget as scoped_ptr, > but I am suggesting it (and overlay_view_) don't have to be. > > I feel they can be raw pointers, and StickyKeysOverlay's dtor can just call > overlay_widget_->Close() which cleans up all the resources used for that widget. > Is there any specific reason to avoid that? I guess my concern is that it's more code to do this if the class doesn't own the widget. What is the benefit of passing ownership of the the widget?
https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... File ash/sticky_keys/sticky_keys_overlay.cc (right): https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay.cc:225: params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET; On 2014/12/05 01:03:14, Tim Song wrote: > On 2014/12/05 00:10:36, Jun Mukai wrote: > > On 2014/12/04 23:53:37, Tim Song wrote: > > > On 2014/12/04 21:58:54, Jun Mukai wrote: > > > > WIDGET_OWNS_NATIVE_WIDGET is not used usually. I believe the standard way > > is: > > > > - the default ownership > > > > - this class observes the widget, and clears the field like > overlay_widget_ > > at > > > > OnWidgetDestroying() > > > > > > I'm using WIDGET_OWNS_NATIVE_WIDGET primarily so if you turn off sticky > keys, > > we > > > can clean up all the resources the feature was using. It seems more > difficult > > to > > > do this without owning the widget. > > > > I am still not so sure what's the problem. > > That sounds like that's because StickyKeysOverlay owns the widget as > scoped_ptr, > > but I am suggesting it (and overlay_view_) don't have to be. > > > > I feel they can be raw pointers, and StickyKeysOverlay's dtor can just call > > overlay_widget_->Close() which cleans up all the resources used for that > widget. > > Is there any specific reason to avoid that? > > I guess my concern is that it's more code to do this if the class doesn't own > the widget. What is the benefit of passing ownership of the the widget? The biggest benefit would be that you don't have to care about view ownership anymore. You can remove the comments and you don't have to care about the order of destruction of the fields in a class. Also I believe the code size would be not so different, just a few lines at most. https://codereview.chromium.org/754763005/diff/100001/ash/root_window_control... File ash/root_window_controller.cc (right): https://codereview.chromium.org/754763005/diff/100001/ash/root_window_control... ash/root_window_controller.cc:166: kShellWindowId_OverlayContainer, }; Oops, I've missed this, but does this work well with other overlay widgets? For example, PartialScreenshotView creates its view for all displays when it's started, therefore it doesn't have to move. Not sure if this is harmless or not. Could you verify that? Ctrl-Shift-F5 to start partial screenshot session.
On 2014/12/05 02:23:08, Jun Mukai wrote: > https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... > File ash/sticky_keys/sticky_keys_overlay.cc (right): > > https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... > ash/sticky_keys/sticky_keys_overlay.cc:225: params.ownership = > views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET; > On 2014/12/05 01:03:14, Tim Song wrote: > > On 2014/12/05 00:10:36, Jun Mukai wrote: > > > On 2014/12/04 23:53:37, Tim Song wrote: > > > > On 2014/12/04 21:58:54, Jun Mukai wrote: > > > > > WIDGET_OWNS_NATIVE_WIDGET is not used usually. I believe the standard > way > > > is: > > > > > - the default ownership > > > > > - this class observes the widget, and clears the field like > > overlay_widget_ > > > at > > > > > OnWidgetDestroying() > > > > > > > > I'm using WIDGET_OWNS_NATIVE_WIDGET primarily so if you turn off sticky > > keys, > > > we > > > > can clean up all the resources the feature was using. It seems more > > difficult > > > to > > > > do this without owning the widget. > > > > > > I am still not so sure what's the problem. > > > That sounds like that's because StickyKeysOverlay owns the widget as > > scoped_ptr, > > > but I am suggesting it (and overlay_view_) don't have to be. > > > > > > I feel they can be raw pointers, and StickyKeysOverlay's dtor can just call > > > overlay_widget_->Close() which cleans up all the resources used for that > > widget. > > > Is there any specific reason to avoid that? > > > > I guess my concern is that it's more code to do this if the class doesn't own > > the widget. What is the benefit of passing ownership of the the widget? > > The biggest benefit would be that you don't have to care about view ownership > anymore. You can remove the comments and you don't have to care about the order > of destruction of the fields in a class. Also I believe the code size would be > not so different, just a few lines at most. > To be clear, it's up to you. The code looks good. Please address my comment on root_window_controller. > https://codereview.chromium.org/754763005/diff/100001/ash/root_window_control... > File ash/root_window_controller.cc (right): > > https://codereview.chromium.org/754763005/diff/100001/ash/root_window_control... > ash/root_window_controller.cc:166: kShellWindowId_OverlayContainer, }; > Oops, I've missed this, but does this work well with other overlay widgets? > For example, PartialScreenshotView creates its view for all displays when it's > started, therefore it doesn't have to move. Not sure if this is harmless or not. > Could you verify that? Ctrl-Shift-F5 to start partial screenshot session.
https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... File ash/sticky_keys/sticky_keys_overlay.cc (right): https://codereview.chromium.org/754763005/diff/80001/ash/sticky_keys/sticky_k... ash/sticky_keys/sticky_keys_overlay.cc:225: params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET; On 2014/12/05 02:23:07, Jun Mukai wrote: > On 2014/12/05 01:03:14, Tim Song wrote: > > On 2014/12/05 00:10:36, Jun Mukai wrote: > > > On 2014/12/04 23:53:37, Tim Song wrote: > > > > On 2014/12/04 21:58:54, Jun Mukai wrote: > > > > > WIDGET_OWNS_NATIVE_WIDGET is not used usually. I believe the standard > way > > > is: > > > > > - the default ownership > > > > > - this class observes the widget, and clears the field like > > overlay_widget_ > > > at > > > > > OnWidgetDestroying() > > > > > > > > I'm using WIDGET_OWNS_NATIVE_WIDGET primarily so if you turn off sticky > > keys, > > > we > > > > can clean up all the resources the feature was using. It seems more > > difficult > > > to > > > > do this without owning the widget. > > > > > > I am still not so sure what's the problem. > > > That sounds like that's because StickyKeysOverlay owns the widget as > > scoped_ptr, > > > but I am suggesting it (and overlay_view_) don't have to be. > > > > > > I feel they can be raw pointers, and StickyKeysOverlay's dtor can just call > > > overlay_widget_->Close() which cleans up all the resources used for that > > widget. > > > Is there any specific reason to avoid that? > > > > I guess my concern is that it's more code to do this if the class doesn't own > > the widget. What is the benefit of passing ownership of the the widget? > > The biggest benefit would be that you don't have to care about view ownership > anymore. You can remove the comments and you don't have to care about the order > of destruction of the fields in a class. Also I believe the code size would be > not so different, just a few lines at most. I feel after being burned already, keeping ownership of both the widget and the view bound to this object is the safest thing to do. It's not easy to reason about the lifetime otherwise. https://codereview.chromium.org/754763005/diff/100001/ash/root_window_control... File ash/root_window_controller.cc (right): https://codereview.chromium.org/754763005/diff/100001/ash/root_window_control... ash/root_window_controller.cc:166: kShellWindowId_OverlayContainer, }; On 2014/12/05 02:23:08, Jun Mukai wrote: > Oops, I've missed this, but does this work well with other overlay widgets? > For example, PartialScreenshotView creates its view for all displays when it's > started, therefore it doesn't have to move. Not sure if this is harmless or not. > Could you verify that? Ctrl-Shift-F5 to start partial screenshot session. I tested this manually, and there doesn't seem to be any problems after the PartialScreenshotView is reparented.
lgtm
ASAN bots are green. Committing again.
The CQ bit was checked by tengs@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/754763005/100001
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/f98986c03fb7047364b9e8bd8c3f74622a315802 Cr-Commit-Position: refs/heads/master@{#307140} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
