Remove the WebContentsObserver::OnMessageReceived overload that doesn't take a RenderFrameHost |
|||
Issue description
There is a TODO in the code saying:
// TODO(nick, creis): Replace all uses of this variant of OnMessageReceived
// with the version that takes a RenderFrameHost, and delete it.
Let's use this bug to track addressing this TODO.
,
Aug 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/89f2c1be46c14a33d4a484f414af8b6dd4934159 commit 89f2c1be46c14a33d4a484f414af8b6dd4934159 Author: Lukasz Anforowicz <lukasza@chromium.org> Date: Wed Aug 30 21:36:46 2017 Use frame-specific WebContentsObserver::OnMessageReceived. The 3 IPC messages touched by this CL are 1) already being sent with a routing id coming from RenderFrameObserver::routing_id. 2) already handled by a frame-specific WebContentsObserver::OnMessageReceived implemented by PluginObserver or PdfPluginPlaceholder Therefore, it seems that the handlers of these IPCs in ChromeWebViewPermissionHelperDelegate are incorrect to listen for this IPCs on the view-related, not frame-specific OnMessageReceived overload. This CL moves handling of these IPCs to be consistently done through frame-specific OnMessageReceived and removes the no-longer-needed overload where possible (e.g. in WebViewPermissionHelper). Bug: 758026 Change-Id: I62c42ee54f4ed12d3076a27ab8093cb1c30fd665 Reviewed-on: https://chromium-review.googlesource.com/627341 Reviewed-by: Lucas Gadani <lfg@chromium.org> Reviewed-by: Nick Carter <nick@chromium.org> Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org> Cr-Commit-Position: refs/heads/master@{#498618} [modify] https://crrev.com/89f2c1be46c14a33d4a484f414af8b6dd4934159/chrome/browser/guest_view/web_view/chrome_web_view_permission_helper_delegate.cc [modify] https://crrev.com/89f2c1be46c14a33d4a484f414af8b6dd4934159/chrome/browser/guest_view/web_view/chrome_web_view_permission_helper_delegate.h [modify] https://crrev.com/89f2c1be46c14a33d4a484f414af8b6dd4934159/content/browser/web_contents/web_contents_impl.cc [modify] https://crrev.com/89f2c1be46c14a33d4a484f414af8b6dd4934159/extensions/browser/guest_view/web_view/web_view_permission_helper.cc [modify] https://crrev.com/89f2c1be46c14a33d4a484f414af8b6dd4934159/extensions/browser/guest_view/web_view/web_view_permission_helper.h
,
Aug 30 2017
This TODO should be in the WebContentsObserver interface too, then, as that's the public interface.
,
Aug 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cf0a94acf49b961d8ea74b9d43363d56ac7d9a6c commit cf0a94acf49b961d8ea74b9d43363d56ac7d9a6c Author: Lukasz Anforowicz <lukasza@chromium.org> Date: Thu Aug 31 19:10:09 2017 Remove unnecessary OnMessageReceived overload from RenderFrameHostTracker. The UIThreadExtensionFunction::RenderFrameHostTracker is a WebContentsObserver that forwards IPC messages to UIThreadExtensionFunction::OnMessageReceived. There is only one overload of UIThreadExtensionFunction::OnMessageReceived - PageCaptureSaveAsMHTMLFunction::OnMessageReceived and the only IPC it handles (ExtensionHostMsg_ResponseAck) is always sent via RenderFrame (in PageCaptureCustomBindings::SendResponseAck). Therefore, we can safely remove the RenderFrameHostTracker::OnMessageReceived overload that doesn't specify the RenderFrameHost* sender. Bug: 758026 Change-Id: If323ca7c42aafe0e53c6d0268ba66229fb622b23 Reviewed-on: https://chromium-review.googlesource.com/644509 Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org> Cr-Commit-Position: refs/heads/master@{#498945} [modify] https://crrev.com/cf0a94acf49b961d8ea74b9d43363d56ac7d9a6c/content/public/browser/web_contents_observer.h [modify] https://crrev.com/cf0a94acf49b961d8ea74b9d43363d56ac7d9a6c/extensions/browser/extension_function.cc
,
Sep 3
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 5
This still needs to be cleaned-up. According to code search the remaining overrides of the *RFH-less* OnMessageReceived are: - BrowserPluginGuest::OnMessageReceived (handling IPCs like ViewHostMsg_LockMouse, ViewHostMsg_TakeFocus, ViewHostMsg_ShowWidget, ...) - BlinkTestController::OnMessageReceived (handling LayoutTest IPCs) - DeclarativeContentCssConditionTracker::PerWebContentsTracker::OnMessageReceived (handling a single ExtensionHostMsg_OnWatchedPageChange IPC) - PrintPreviewObserver::OnMessageReceived (test-only implementation that handles PrintHostMsg_DidStartPreview) |
|||
►
Sign in to add a comment |
|||
Comment 1 by lukasza@chromium.org
, Aug 22 2017