Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1842733002:
Handle null SuggestionsService in chrome://suggestions (Closed)
|
Created:
4 years, 8 months ago by Marc Treib Modified:
4 years, 8 months ago Reviewers:
Bernhard Bauer CC:
chromium-reviews, skanuj+watch_chromium.org, melevin+watch_chromium.org, dhollowa+watch_chromium.org, dougw+watch_chromium.org, donnd+watch_chromium.org, jfweitz+watch_chromium.org, David Black, samarth+watch_chromium.org, kmadhusu+watch_chromium.org, Jered Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionHandle null SuggestionsService in chrome://suggestions
SuggestionsService can be null in guest profiles, so we return the fallback value in that case.
BUG=598253
Committed: https://crrev.com/1992d036cbc12e6e0131fafd7a5e7d62d5105098
Cr-Commit-Position: refs/heads/master@{#383707}
Patch Set 1 #
Total comments: 2
Messages
Total messages: 15 (7 generated)
The CQ bit was checked by treib@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/1842733002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1842733002/1
treib@chromium.org changed reviewers: + bauerb@chromium.org
https://codereview.chromium.org/1842733002/diff/1/chrome/browser/search/sugge... File chrome/browser/search/suggestions/suggestions_source.cc (right): https://codereview.chromium.org/1842733002/diff/1/chrome/browser/search/sugge... chrome/browser/search/suggestions/suggestions_source.cc:52: std::string RenderOutputHtml( Unrelated cleanup: With C++11, there's no need to use an out param anymore :)
LGTM, but a small commit message nit: Your commit message doesn't really say why you're not crashing anymore (it only says you are not crashing, but that should be implicit from the linked bug -- hopefully you're not adding more crashes! 😃) So, I would instead explain what your change exactly does, in a way that makes the connection between the change and the fixed bug, like: > Handle null SuggestionsService in chrome://suggestions > > SuggestionsService can be null in guest profiles, so we return the fallback value in that case. > > BUG=598253 https://codereview.chromium.org/1842733002/diff/1/chrome/browser/search/sugge... File chrome/browser/search/suggestions/suggestions_source.cc (right): https://codereview.chromium.org/1842733002/diff/1/chrome/browser/search/sugge... chrome/browser/search/suggestions/suggestions_source.cc:52: std::string RenderOutputHtml( On 2016/03/29 12:14:26, Marc Treib wrote: > Unrelated cleanup: With C++11, there's no need to use an out param anymore :) Hooray!
Description was changed from ========== chrome://suggestions: don't crash in guest profiles BUG=598253 ========== to ========== Handle null SuggestionsService in chrome://suggestions SuggestionsService can be null in guest profiles, so we return the fallback value in that case. BUG=598253 ==========
On 2016/03/29 12:31:48, Bernhard Bauer wrote: > LGTM, but a small commit message nit: Your commit message doesn't really say why > you're not crashing anymore (it only says you are not crashing, but that should > be implicit from the linked bug -- hopefully you're not adding more crashes! 😃) > > So, I would instead explain what your change exactly does, in a way that makes > the connection between the change and the fixed bug, like: > > > Handle null SuggestionsService in chrome://suggestions > > > > SuggestionsService can be null in guest profiles, so we return the fallback > value in that case. > > > > BUG=598253 Thanks for the suggestion, done!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by treib@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1842733002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1842733002/1
Message was sent while issue was closed.
Description was changed from ========== Handle null SuggestionsService in chrome://suggestions SuggestionsService can be null in guest profiles, so we return the fallback value in that case. BUG=598253 ========== to ========== Handle null SuggestionsService in chrome://suggestions SuggestionsService can be null in guest profiles, so we return the fallback value in that case. BUG=598253 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Handle null SuggestionsService in chrome://suggestions SuggestionsService can be null in guest profiles, so we return the fallback value in that case. BUG=598253 ========== to ========== Handle null SuggestionsService in chrome://suggestions SuggestionsService can be null in guest profiles, so we return the fallback value in that case. BUG=598253 Committed: https://crrev.com/1992d036cbc12e6e0131fafd7a5e7d62d5105098 Cr-Commit-Position: refs/heads/master@{#383707} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/1992d036cbc12e6e0131fafd7a5e7d62d5105098 Cr-Commit-Position: refs/heads/master@{#383707} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
