Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2904993002:
Call IdleSpellCheckCallback::Deactivate() in PrepareForLeakDetection() (Closed)
DescriptionCall IdleSpellCheckCallback::Deactivate() in PrepareForLeakDetection()
IdleSpellCheckCallback's cold mode timer can be fired and thus
ScriptedIdleTaskController can be created after
PrepareForLeakDetection() before leak detection, causing
ScriptedIdleTaskController to be reported as leaking.
This CL fixes this by calling Deactivate().
Tests for this issue is not included because this is quite
timing-dependent and is hard to reproduce reliably.
BUG=718114
Review-Url: https://codereview.chromium.org/2904993002
Cr-Commit-Position: refs/heads/master@{#475302}
Committed: https://chromium.googlesource.com/chromium/src/+/7b01675740f7b8841d4b59c4a35b517bb19dd401
Patch Set 1 #
Total comments: 2
Patch Set 2 : Add runtime flag guard #Messages
Total messages: 22 (15 generated)
The CQ bit was checked by hiroshige@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: This issue passed the CQ dry run.
Description was changed from ========== Call IdleSpellCheckCallback::Deactivate() in PrepareForLeakDetection() BUG= ========== to ========== Call IdleSpellCheckCallback::Deactivate() in PrepareForLeakDetection() BUG=718114 ==========
hiroshige@chromium.org changed reviewers: + xiaochengh@chromium.org
Description was changed from ========== Call IdleSpellCheckCallback::Deactivate() in PrepareForLeakDetection() BUG=718114 ========== to ========== Call IdleSpellCheckCallback::Deactivate() in PrepareForLeakDetection() IdleSpellCheckCallback's cold mode timer can be fired and thus ScriptedIdleTaskController can be created after PrepareForLeakDetection() before leak detection, causing ScriptedIdleTaskController to be reported as leaking. This CL fixes this by calling Deactivate(). Tests for this issue is not included because this is quite timing-dependent and is hard to reproduce reliably. BUG=718114 ==========
PTAL.
Thanks for the patch! https://codereview.chromium.org/2904993002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp (right): https://codereview.chromium.org/2904993002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp:1081: idle_spell_check_callback_->Deactivate(); Please guard this line with if (RuntimeEnabledFeatures::idleTimeSpellCheckingEnabled())
The CQ bit was checked by hiroshige@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...
https://codereview.chromium.org/2904993002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp (right): https://codereview.chromium.org/2904993002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp:1081: idle_spell_check_callback_->Deactivate(); On 2017/05/26 21:15:09, Xiaocheng wrote: > Please guard this line with > > if (RuntimeEnabledFeatures::idleTimeSpellCheckingEnabled()) Done.
Thanks, lgtm. I'm not an owner of this directory. Please find an owner for approval...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
kouhei@chromium.org changed reviewers: + kouhei@chromium.org
The CQ bit was checked by kouhei@chromium.org
lgtm core lgtm
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": 20001, "attempt_start_ts": 1496035801104080,
"parent_rev": "11378e7149c40c9f930127e2db7d2f1e19c32b23", "commit_rev":
"7b01675740f7b8841d4b59c4a35b517bb19dd401"}
Message was sent while issue was closed.
Description was changed from ========== Call IdleSpellCheckCallback::Deactivate() in PrepareForLeakDetection() IdleSpellCheckCallback's cold mode timer can be fired and thus ScriptedIdleTaskController can be created after PrepareForLeakDetection() before leak detection, causing ScriptedIdleTaskController to be reported as leaking. This CL fixes this by calling Deactivate(). Tests for this issue is not included because this is quite timing-dependent and is hard to reproduce reliably. BUG=718114 ========== to ========== Call IdleSpellCheckCallback::Deactivate() in PrepareForLeakDetection() IdleSpellCheckCallback's cold mode timer can be fired and thus ScriptedIdleTaskController can be created after PrepareForLeakDetection() before leak detection, causing ScriptedIdleTaskController to be reported as leaking. This CL fixes this by calling Deactivate(). Tests for this issue is not included because this is quite timing-dependent and is hard to reproduce reliably. BUG=718114 Review-Url: https://codereview.chromium.org/2904993002 Cr-Commit-Position: refs/heads/master@{#475302} Committed: https://chromium.googlesource.com/chromium/src/+/7b01675740f7b8841d4b59c4a35b... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/7b01675740f7b8841d4b59c4a35b... |
