New issue
Advanced search Search tips

Issue 769751 link

Starred by 1 user
Status: Fixed
Owner:
yhanada@chromium.org
Closed: Oct 2017
Cc:
yukishiino@chromium.org
shuchen@chromium.org
m...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Dead code clean-up opportunity near |WebView::observing_render_process_host_|

Project Member Reported by lukasza@chromium.org, Sep 28 2017 
Paraphrasing what yukishiino@ pointed out in https://chromium-review.googlesource.com/c/chromium/src/+/689016 :

We shouldn't need to observe the lifetime of the render process host.   It was once important, but shuchen@ did a great work on text input focus, and retired View::GetTextInputClient ([1], [2]).  As my code discussed here was introduced to support View::GetTextInputClient, it's no longer needed.  This is a kind of leftover of the refactoring of text input focus.

[1] https://codereview.chromium.org/1177503003
[2] https://codereview.chromium.org/1182523003

Comment 1 by lukasza@chromium.org, Sep 28 2017

Cc: -yhanada@chromium.org
Owner: yhanada@chromium.org
Assigning to yhanada@ for further triage.  My apologies if I assigned this bug to a wrong component or didn't get all details quite right - I am just trying to open a bug to ensure that we won't miss or forget the refactoring / dead-code-deleting opportunity pointed out in https://chromium-review.googlesource.com/c/chromium/src/+/689016.  FWIW, I am not very familiar with the code in question :-(

Comment 2 by yhanada@chromium.org, Oct 3 2017

Status: Assigned (was: Untriaged)
No worries. I'll take a look at this.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 11 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4219742abdc17e4288fca7f96c6be3ae06e17680

commit 4219742abdc17e4288fca7f96c6be3ae06e17680
Author: Yuichiro Hanada <yhanada@chromium.org>
Date: Wed Oct 11 22:54:24 2017

Delete dead code in views::WebView.

WebView observes the lifetime of the render process host to support
View::GetTextInputClient, but it has been removed.
RenderProcessHostObserver::RenderProcessExited() is still used to call
NotifyAccessibilityWebContentsChanged(), but it can be replaced by
WebContentsObserver::RenderProcessGone().

Bug:  769751 
Change-Id: Ia9d41d4f679401495a6ebd6730035d8e28558b03
Reviewed-on: https://chromium-review.googlesource.com/712954
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508152}
[modify] https://crrev.com/4219742abdc17e4288fca7f96c6be3ae06e17680/ui/views/controls/webview/webview.cc
[modify] https://crrev.com/4219742abdc17e4288fca7f96c6be3ae06e17680/ui/views/controls/webview/webview.h

Comment 4 by yhanada@chromium.org, Oct 11 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment