Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1994253004:
Speculative ShelfView::GetTitleForView crash fix. (Closed)
DescriptionSpeculative ShelfView::GetTitleForView crash fix.
Return empty string if any needed values are null.
BUG=612042
TEST=No crash repro (I can't repro with or without this CL...)
R=jamescook@chromium.org,sky@chromium.org
Committed: https://crrev.com/c24961f458d8016232ab0bff0248e9131c5c7046
Cr-Commit-Position: refs/heads/master@{#394896}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Remove unnecessary check. #Messages
Total messages: 18 (8 generated)
Hey James, please take a look; thanks!
The CQ bit was checked by msw@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/1994253004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1994253004/1
LGTM It would be nice to know the real cause, but given that this happens in beta I think landing this fix is a good idea. It would be good to get it into 52, which I think branches today.
Description was changed from ========== Speculative ShelfView::GetTitleForView crash fix. Return empty string if any needed values are null. BUG=612042 TEST=No crash repro (I can't repro with or without this CL...) R=jamescook@chromium.org ========== to ========== Speculative ShelfView::GetTitleForView crash fix. Return empty string if any needed values are null. BUG=612042 TEST=No crash repro (I can't repro with or without this CL...) R=jamescook@chromium.org,sky@chromium.org ==========
msw@chromium.org changed reviewers: + sky@chromium.org
+Scott, please take a look for OWNERS, thanks!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM https://codereview.chromium.org/1994253004/diff/1/ash/shelf/shelf_view.cc File ash/shelf/shelf_view.cc (right): https://codereview.chromium.org/1994253004/diff/1/ash/shelf/shelf_view.cc#new... ash/shelf/shelf_view.cc:545: if (!item || !item_manager_ || !item_manager_->GetShelfItemDelegate(item->id)) The item_manager_ check seems unnecessary.
https://codereview.chromium.org/1994253004/diff/1/ash/shelf/shelf_view.cc File ash/shelf/shelf_view.cc (right): https://codereview.chromium.org/1994253004/diff/1/ash/shelf/shelf_view.cc#new... ash/shelf/shelf_view.cc:545: if (!item || !item_manager_ || !item_manager_->GetShelfItemDelegate(item->id)) On 2016/05/19 20:54:48, sky wrote: > The item_manager_ check seems unnecessary. Done.
The CQ bit was checked by msw@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/1994253004/#ps20001 (title: "Remove unnecessary check.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1994253004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1994253004/20001
Message was sent while issue was closed.
Description was changed from ========== Speculative ShelfView::GetTitleForView crash fix. Return empty string if any needed values are null. BUG=612042 TEST=No crash repro (I can't repro with or without this CL...) R=jamescook@chromium.org,sky@chromium.org ========== to ========== Speculative ShelfView::GetTitleForView crash fix. Return empty string if any needed values are null. BUG=612042 TEST=No crash repro (I can't repro with or without this CL...) R=jamescook@chromium.org,sky@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Speculative ShelfView::GetTitleForView crash fix. Return empty string if any needed values are null. BUG=612042 TEST=No crash repro (I can't repro with or without this CL...) R=jamescook@chromium.org,sky@chromium.org ========== to ========== Speculative ShelfView::GetTitleForView crash fix. Return empty string if any needed values are null. BUG=612042 TEST=No crash repro (I can't repro with or without this CL...) R=jamescook@chromium.org,sky@chromium.org Committed: https://crrev.com/c24961f458d8016232ab0bff0248e9131c5c7046 Cr-Commit-Position: refs/heads/master@{#394896} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/c24961f458d8016232ab0bff0248e9131c5c7046 Cr-Commit-Position: refs/heads/master@{#394896} |
