Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(34)

Issue 2813493003: Fix a crash on Mac due to TextInputManager being null (Closed)

Created:
3 years, 8 months ago by EhsanK
Modified:
3 years, 8 months ago
Reviewers:
Avi (use Gerrit)
CC:
chromium-reviews, yusukes+watch_chromium.org, shuchen+watch_chromium.org, jam, nona+watch_chromium.org, darin-cc_chromium.org, mac-reviews_chromium.org, James Su
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix a crash on Mac due to TextInputManager being null The reference to TextInputManager instance (owned by WebContentsImpl) in RenderWidgetHostView is alive as long as 1) WebContents is alive, 2) RenderWidgetHostView is registered with TextInputManager. On Mac, WebContents destruction might not lead to immediate destruction of the RenderWidgetHostViewMac. Therefore, there can be a period of time where (after WebContentsImpl is deleted) |text_input_manager_| is null. Then it is only safe to check |text_input_manager_| before dereferencing it. BUG=709864 Review-Url: https://codereview.chromium.org/2813493003 Cr-Commit-Position: refs/heads/master@{#463338} Committed: https://chromium.googlesource.com/chromium/src/+/f50104bb610bec50956cfe363d08c9682b930ab9

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -1 line) Patch
M content/browser/renderer_host/render_widget_host_view_mac.mm View 1 chunk +2 lines, -1 line 1 comment Download

Messages

Total messages: 13 (8 generated)
EhsanK
Hello Avi, PTAL. Thanks! https://codereview.chromium.org/2813493003/diff/1/content/browser/renderer_host/render_widget_host_view_mac.mm File content/browser/renderer_host/render_widget_host_view_mac.mm (right): https://codereview.chromium.org/2813493003/diff/1/content/browser/renderer_host/render_widget_host_view_mac.mm#newcode567 content/browser/renderer_host/render_widget_host_view_mac.mm:567: return GetTextInputManager() ? text_input_manager_->GetCompositionRangeInfo() This ...
3 years, 8 months ago (2017-04-10 15:30:39 UTC) #4
Avi (use Gerrit)
LGTM
3 years, 8 months ago (2017-04-10 18:22:09 UTC) #7
EhsanK
Thanks for the quick review even when OOO :)! I will CQ soon. Hopefully stopping ...
3 years, 8 months ago (2017-04-10 18:24:05 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2813493003/1
3 years, 8 months ago (2017-04-10 18:25:00 UTC) #10
commit-bot: I haz the power
3 years, 8 months ago (2017-04-10 18:36:47 UTC) #13
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/f50104bb610bec50956cfe363d08...

Powered by Google App Engine
This is Rietveld 408576698