New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 604645 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 416660
issue 614742



Sign in to add a comment

Various methods to get focused frames should only return local frames

Project Member Reported by dcheng@chromium.org, Apr 19 2016

Issue description

FocusController::focusedOrMainFrame(): this should only return a frame if the focused frame (or the main frame?) is local to the renderer process. In particular, this should return a *reference* if possible: it should be invalid to call this in a process if the focused/main frame is not local.

WebView::focusedFrame(): this uses focusedOrMainFrame() internally, so it should have the same semantics. Callers should not null-check the value… probably. There might be some edge cases where this is null around frame detach? It's something to investigate.

I don't think there are any others in Blink, but there might be some things to fix up in //content as well (we've already fixed FocusController::focusedFrame() to use these semantics).

Motivation: in the long-run, this will help us ensure the important invariant that operations that require a DOM are routed to the renderer with the local frame. It will also provide overall simplification of code, with less downcasting from WebFrame to WebLocalFrame.
 

Comment 1 by dcheng@chromium.org, Apr 19 2016

One bit of code that would immediately benefit from doing this is the IME code (as discussed in https://codereview.chromium.org/1889053003#msg15).

Comment 2 by yabinh@chromium.org, Apr 19 2016

Labels: -Pri-1 Pri-3
Owner: yabinh@chromium.org
Status: Assigned (was: Available)

Comment 3 by groby@chromium.org, May 27 2016

Blocking: 614742
Project Member

Comment 4 by sheriffbot@chromium.org, Jun 1 2016

Labels: -M-52 M-53 MovedFrom-52
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 8 2016

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

commit 8204efcf337a42af04aebcee8827026b317071f0
Author: yabinh <yabinh@chromium.org>
Date: Fri Jul 08 13:58:57 2016

Move IME related functions from WebFrame to WebLocalFrame

BUG= 604645 

Review-Url: https://codereview.chromium.org/2012823003
Cr-Commit-Position: refs/heads/master@{#404345}

[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/chrome/renderer/spellchecker/spellcheck_provider.cc
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/components/printing/renderer/print_web_view_helper.cc
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/components/test_runner/test_runner_for_specific_view.cc
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/components/test_runner/text_input_controller.cc
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/content/renderer/render_view_impl.cc
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/content/renderer/text_input_client_observer.cc
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/third_party/WebKit/Source/web/WebRemoteFrameImpl.cpp
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/third_party/WebKit/Source/web/WebRemoteFrameImpl.h
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/third_party/WebKit/Source/web/WebViewImpl.h
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/third_party/WebKit/Source/web/tests/VisualViewportTest.cpp
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/third_party/WebKit/Source/web/tests/WebPluginContainerTest.cpp
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/third_party/WebKit/Source/web/tests/WebViewTest.cpp
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/third_party/WebKit/public/web/WebFrame.h
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/third_party/WebKit/public/web/WebLocalFrame.h
[modify] https://crrev.com/8204efcf337a42af04aebcee8827026b317071f0/third_party/WebKit/public/web/WebView.h

Project Member

Comment 6 by sheriffbot@chromium.org, Jul 13 2016

Labels: -M-53 MovedFrom-53
This issue has been moved once and is lower than Pri-1. Removing the milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 7 by yabinh@chromium.org, Jul 19 2016

Status: Fixed (was: Assigned)
Components: Blink>HTML>Focus
Components: -Blink>Focus

Sign in to add a comment