|
|
Chromium Code Reviews|
Created:
4 years, 2 months ago by timvolodine Modified:
4 years, 2 months ago Reviewers:
Tobias Sargeant CC:
chromium-reviews, groby+spellwatch_chromium.org, rlp+watch_chromium.org, rouslan+spell_chromium.org, timvolodine, agrieve+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Android] Fix NullPointerException crash in SpellCheckerSessionBridge.
Add a check for potential null results when the spellchecking service
returns in onGetSentenceSuggestions.
Haven't tested, reproduced or investigated the cause of this (i.e.
nulls in results) but seems the right approach to avoid crashes in
the first place, see crbug.com/651458 for more details.
BUG=651458, 583616, 629609
Committed: https://crrev.com/f16e0045b2dd3e399e14e5a44db1d8b1b18d46e9
Cr-Commit-Position: refs/heads/master@{#425004}
Patch Set 1 #
Total comments: 2
Patch Set 2 : fix comment #
Messages
Total messages: 18 (12 generated)
The CQ bit was checked by timvolodine@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...
timvolodine@chromium.org changed reviewers: + tobiasjs@chromium.org
ptal, a crash fix as suggested on the bug, we can try to figure out why/when this happens at some later point
lgtm. https://codereview.chromium.org/2413873003/diff/1/components/spellcheck/brows... File components/spellcheck/browser/android/java/src/org/chromium/components/spellcheck/SpellCheckerSessionBridge.java (right): https://codereview.chromium.org/2413873003/diff/1/components/spellcheck/brows... components/spellcheck/browser/android/java/src/org/chromium/components/spellcheck/SpellCheckerSessionBridge.java:104: // In some cases null can be returned by the system, see maybe s/system/selected spellchecking service/ ?
The CQ bit was checked by timvolodine@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 ========== [Android] Fix NullPointerException crash in SpellCheckerSessionBridge. Add a check for potential null results when the spellchecking service returns in onGetSentenceSuggestions. Haven't tested, reproduced or investigated the cause of this (i.e. nulls in results) but seems the right approach to avoid crashes in the first place, see crbug.com/651458 for more details. BUG=651458 ========== to ========== [Android] Fix NullPointerException crash in SpellCheckerSessionBridge. Add a check for potential null results when the spellchecking service returns in onGetSentenceSuggestions. Haven't tested, reproduced or investigated the cause of this (i.e. nulls in results) but seems the right approach to avoid crashes in the first place, see crbug.com/651458 for more details. BUG=651458,583616,629609 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Thanks Toby! https://codereview.chromium.org/2413873003/diff/1/components/spellcheck/brows... File components/spellcheck/browser/android/java/src/org/chromium/components/spellcheck/SpellCheckerSessionBridge.java (right): https://codereview.chromium.org/2413873003/diff/1/components/spellcheck/brows... components/spellcheck/browser/android/java/src/org/chromium/components/spellcheck/SpellCheckerSessionBridge.java:104: // In some cases null can be returned by the system, see On 2016/10/13 11:12:31, Tobias Sargeant wrote: > maybe s/system/selected spellchecking service/ ? Done.
The CQ bit was checked by timvolodine@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tobiasjs@chromium.org Link to the patchset: https://codereview.chromium.org/2413873003/#ps20001 (title: "fix comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [Android] Fix NullPointerException crash in SpellCheckerSessionBridge. Add a check for potential null results when the spellchecking service returns in onGetSentenceSuggestions. Haven't tested, reproduced or investigated the cause of this (i.e. nulls in results) but seems the right approach to avoid crashes in the first place, see crbug.com/651458 for more details. BUG=651458,583616,629609 ========== to ========== [Android] Fix NullPointerException crash in SpellCheckerSessionBridge. Add a check for potential null results when the spellchecking service returns in onGetSentenceSuggestions. Haven't tested, reproduced or investigated the cause of this (i.e. nulls in results) but seems the right approach to avoid crashes in the first place, see crbug.com/651458 for more details. BUG=651458,583616,629609 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [Android] Fix NullPointerException crash in SpellCheckerSessionBridge. Add a check for potential null results when the spellchecking service returns in onGetSentenceSuggestions. Haven't tested, reproduced or investigated the cause of this (i.e. nulls in results) but seems the right approach to avoid crashes in the first place, see crbug.com/651458 for more details. BUG=651458,583616,629609 ========== to ========== [Android] Fix NullPointerException crash in SpellCheckerSessionBridge. Add a check for potential null results when the spellchecking service returns in onGetSentenceSuggestions. Haven't tested, reproduced or investigated the cause of this (i.e. nulls in results) but seems the right approach to avoid crashes in the first place, see crbug.com/651458 for more details. BUG=651458,583616,629609 Committed: https://crrev.com/f16e0045b2dd3e399e14e5a44db1d8b1b18d46e9 Cr-Commit-Position: refs/heads/master@{#425004} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/f16e0045b2dd3e399e14e5a44db1d8b1b18d46e9 Cr-Commit-Position: refs/heads/master@{#425004} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
