Skip to content

Commit

Permalink
Move SetWebUIProperty from RenderViewHost to RenderFrameHost.
Browse files Browse the repository at this point in the history
WebUI has moved to be a per-frame object, but the SetWebUIProperty
method is still on the RenderViewHost object. This CL moves it to be
per-frame, therefore on the RenderFrameHost object, but does not make
it available to subframes. The goal is to be a purely code move CL.

Bug: 763548, 1002276, 1019739
Change-Id: Ie857a253f750316fe090ac2daa6686376cdb1833
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1901587
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713231}
  • Loading branch information
naskooskov authored and Commit Bot committed Nov 6, 2019
1 parent 31dfc58 commit dce1a62
Show file tree
Hide file tree
Showing 18 changed files with 54 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ void AccountMigrationWelcomeUITest::ShowDialog() {
auto* webui = dialog->GetWebUIForTest();
auto* web_contents = webui->GetWebContents();
content::WaitForLoadStop(web_contents);
web_contents->GetRenderViewHost()->SetWebUIProperty(
web_contents->GetMainFrame()->SetWebUIProperty(
"expectedUrl", chrome::kChromeUIAccountMigrationWelcomeURL);
web_contents->GetRenderViewHost()->SetWebUIProperty("expectedEmail",
account_email);
web_contents->GetMainFrame()->SetWebUIProperty("expectedEmail",
account_email);
SetWebUIInstance(webui);
}
8 changes: 4 additions & 4 deletions chrome/browser/ui/webui/constrained_web_dialog_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,14 @@ void ConstrainedWebDialogUI::RenderFrameCreated(
ui::WebDialogDelegate* dialog_delegate = delegate->GetWebDialogDelegate();
std::vector<WebUIMessageHandler*> handlers;
dialog_delegate->GetWebUIMessageHandlers(&handlers);
RenderViewHost* render_view_host = render_frame_host->GetRenderViewHost();
render_view_host->SetWebUIProperty("dialogArguments",
dialog_delegate->GetDialogArgs());
render_frame_host->SetWebUIProperty("dialogArguments",
dialog_delegate->GetDialogArgs());
for (WebUIMessageHandler* handler : handlers) {
web_ui()->AddMessageHandler(base::WrapUnique(handler));
}

dialog_delegate->OnDialogShown(web_ui(), render_view_host);
dialog_delegate->OnDialogShown(web_ui(),
render_frame_host->GetRenderViewHost());
}

void ConstrainedWebDialogUI::OnDialogCloseMessage(const base::ListValue* args) {
Expand Down
2 changes: 1 addition & 1 deletion chrome/test/data/webui/certificate_viewer_ui_test-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ void CertificateViewerUITest::ShowCertificateViewer() {
content::WebContents* webui_webcontents = dialog->webui_->GetWebContents();
content::WaitForLoadStop(webui_webcontents);
content::WebUI* webui = webui_webcontents->GetWebUI();
webui_webcontents->GetRenderViewHost()->SetWebUIProperty(
webui_webcontents->GetMainFrame()->SetWebUIProperty(
"expectedUrl", chrome::kChromeUICertificateViewerURL);
SetWebUIInstance(webui);
}
Expand Down
12 changes: 12 additions & 0 deletions content/browser/frame_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3229,6 +3229,18 @@ int RenderFrameHostImpl::GetEnabledBindings() {
return enabled_bindings_;
}

void RenderFrameHostImpl::SetWebUIProperty(const std::string& name,
const std::string& value) {
// This is a sanity check before telling the renderer to enable the property.
// It could lie and send the corresponding IPC messages anyway, but we will
// not act on them if enabled_bindings_ doesn't agree. If we get here without
// WebUI bindings, terminate the renderer process.
if (enabled_bindings_ & BINDINGS_POLICY_WEB_UI)
Send(new FrameMsg_SetWebUIProperty(routing_id_, name, value));
else
ReceivedBadMessage(GetProcess(), bad_message::RVH_WEB_UI_BINDINGS_MISMATCH);
}

void RenderFrameHostImpl::DisableBeforeUnloadHangMonitorForTesting() {
beforeunload_timeout_.reset();
}
Expand Down
2 changes: 2 additions & 0 deletions content/browser/frame_host/render_frame_host_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,8 @@ class CONTENT_EXPORT RenderFrameHostImpl
const std::string& message) override;
void AllowBindings(int binding_flags) override;
int GetEnabledBindings() override;
void SetWebUIProperty(const std::string& name,
const std::string& value) override;
void DisableBeforeUnloadHangMonitorForTesting() override;
bool IsBeforeUnloadHangMonitorDisabledForTesting() override;
bool GetSuddenTerminationDisablerState(
Expand Down
12 changes: 0 additions & 12 deletions content/browser/renderer_host/render_view_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -764,18 +764,6 @@ RenderFrameHost* RenderViewHostImpl::GetMainFrame() {
return delegate_->GetPendingMainFrame();
}

void RenderViewHostImpl::SetWebUIProperty(const std::string& name,
const std::string& value) {
// This is a sanity check before telling the renderer to enable the property.
// It could lie and send the corresponding IPC messages anyway, but we will
// not act on them if enabled_bindings_ doesn't agree. If we get here without
// WebUI bindings, kill the renderer process.
if (GetMainFrame()->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI)
Send(new ViewMsg_SetWebUIProperty(GetRoutingID(), name, value));
else
ReceivedBadMessage(GetProcess(), bad_message::RVH_WEB_UI_BINDINGS_MISMATCH);
}

void RenderViewHostImpl::RenderWidgetGotFocus() {
RenderViewHostDelegateView* view = delegate_->GetDelegateView();
if (view)
Expand Down
2 changes: 0 additions & 2 deletions content/browser/renderer_host/render_view_host_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,6 @@ class CONTENT_EXPORT RenderViewHostImpl
SiteInstanceImpl* GetSiteInstance() override;
bool IsRenderViewLive() override;
void NotifyMoveOrResizeStarted() override;
void SetWebUIProperty(const std::string& name,
const std::string& value) override;
WebPreferences GetWebkitPreferences() override;
void UpdateWebkitPreferences(const WebPreferences& prefs) override;
void OnWebkitPreferencesChanged() override;
Expand Down
4 changes: 2 additions & 2 deletions content/browser/security_exploit_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -408,8 +408,8 @@ IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest, SetWebUIProperty) {
EXPECT_EQ(base::ASCIIToUTF16("OK"), shell()->web_contents()->GetTitle());
EXPECT_EQ(0, shell()->web_contents()->GetMainFrame()->GetEnabledBindings());

RenderViewHost* compromised_renderer =
shell()->web_contents()->GetRenderViewHost();
RenderFrameHost* compromised_renderer =
shell()->web_contents()->GetMainFrame();
RenderProcessHostKillWaiter kill_waiter(compromised_renderer->GetProcess());
compromised_renderer->SetWebUIProperty("toolkit", "views");
EXPECT_EQ(bad_message::RVH_WEB_UI_BINDINGS_MISMATCH, kill_waiter.Wait());
Expand Down
6 changes: 6 additions & 0 deletions content/common/frame_messages.h
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,12 @@ IPC_MESSAGE_ROUTED2(FrameMsg_MediaPlayerActionAt,
// fallback contents (i.e., <object>).
IPC_MESSAGE_ROUTED0(FrameMsg_RenderFallbackContent)

// Tell the renderer to add a property to the WebUI binding object. This
// only works if we allowed WebUI bindings.
IPC_MESSAGE_ROUTED2(FrameMsg_SetWebUIProperty,
std::string /* property_name */,
std::string /* property_value_json */)

// -----------------------------------------------------------------------------
// Messages sent from the renderer to the browser.

Expand Down
6 changes: 0 additions & 6 deletions content/common/view_messages.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,6 @@ IPC_MESSAGE_ROUTED2(ViewMsg_PluginActionAt,
// Sets the page scale for the current main frame to the given page scale.
IPC_MESSAGE_ROUTED1(ViewMsg_SetPageScale, float /* page_scale_factor */)

// Tell the renderer to add a property to the WebUI binding object. This
// only works if we allowed WebUI bindings.
IPC_MESSAGE_ROUTED2(ViewMsg_SetWebUIProperty,
std::string /* property_name */,
std::string /* property_value_json */)

// Used to notify the render-view that we have received a target URL. Used
// to prevent target URLs spamming the browser.
IPC_MESSAGE_ROUTED0(ViewMsg_UpdateTargetURL_ACK)
Expand Down
5 changes: 5 additions & 0 deletions content/public/browser/render_frame_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,11 @@ class CONTENT_EXPORT RenderFrameHost : public IPC::Listener,
// RenderFrame. See BindingsPolicy for details.
virtual int GetEnabledBindings() = 0;

// Sets a property with the given name and value on the WebUI object
// associated with this RenderFrameHost, if one exists.
virtual void SetWebUIProperty(const std::string& name,
const std::string& value) = 0;

#if defined(OS_ANDROID)
// Returns an InterfaceProvider for Java-implemented interfaces that are
// scoped to this RenderFrameHost. This provides access to interfaces
Expand Down
5 changes: 0 additions & 5 deletions content/public/browser/render_view_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,6 @@ class CONTENT_EXPORT RenderViewHost : public IPC::Sender {
// started.
virtual void NotifyMoveOrResizeStarted() = 0;

// Sets a property with the given name and value on the Web UI binding object.
// Must call AllowWebUIBindings() on this renderer first.
virtual void SetWebUIProperty(const std::string& name,
const std::string& value) = 0;

// TODO(mustaq): Replace "Webkit" from the following three method names.
//
// Returns the current WebKit preferences. Note: WebPreferences is cached, so
Expand Down
7 changes: 4 additions & 3 deletions content/renderer/render_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
#include "content/public/renderer/render_frame_observer.h"
#include "content/public/renderer/render_frame_visitor.h"
#include "content/public/renderer/render_thread.h"
#include "content/public/renderer/render_view_observer.h"
#include "content/public/renderer/renderer_ppapi_host.h"
#include "content/renderer/accessibility/aom_content_ax_tree.h"
#include "content/renderer/accessibility/render_accessibility_impl.h"
Expand Down Expand Up @@ -3246,11 +3247,11 @@ void RenderFrameImpl::CancelBlockedRequests() {
}

void RenderFrameImpl::AllowBindings(int32_t enabled_bindings_flags) {
// TODO(nasko): WebUIExtensionsData might be useful to be registered for
// subframes as well, though at this time there is no such usage.
if (IsMainFrame() && (enabled_bindings_flags & BINDINGS_POLICY_WEB_UI) &&
!(enabled_bindings_ & BINDINGS_POLICY_WEB_UI)) {
// TODO(sammc): Move WebUIExtensionData to be a RenderFrameObserver.
// WebUIExtensionData deletes itself when |render_view_| is destroyed.
new WebUIExtensionData(render_view_);
new WebUIExtensionData(this);
}

enabled_bindings_ |= enabled_bindings_flags;
Expand Down
1 change: 0 additions & 1 deletion content/renderer/render_view_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,6 @@ class CONTENT_EXPORT RenderViewImpl : public blink::WebViewClient,
void OnSetInitialFocus(bool reverse);
void OnSetRendererPrefs(
const blink::mojom::RendererPreferences& renderer_prefs);
void OnSetWebUIProperty(const std::string& name, const std::string& value);
void OnSuppressDialogsUntilSwapOut();
void OnUpdateTargetURLAck();
void OnUpdateWebPreferences(const WebPreferences& prefs);
Expand Down
2 changes: 1 addition & 1 deletion content/renderer/web_ui_extension.cc
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ std::string WebUIExtension::GetVariableValue(const std::string& name) {
if (!ShouldRespondToRequest(&frame, &render_frame))
return std::string();

return WebUIExtensionData::Get(render_frame->GetRenderView())->GetValue(name);
return WebUIExtensionData::Get(render_frame)->GetValue(name);
}

} // namespace content
13 changes: 6 additions & 7 deletions content/renderer/web_ui_extension_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,14 @@

#include "content/renderer/web_ui_extension_data.h"

#include "content/common/view_messages.h"
#include "content/public/renderer/render_view.h"
#include "content/common/frame_messages.h"
#include "content/public/renderer/render_frame.h"

namespace content {

WebUIExtensionData::WebUIExtensionData(RenderView* render_view)
: RenderViewObserver(render_view),
RenderViewObserverTracker<WebUIExtensionData>(render_view) {
}
WebUIExtensionData::WebUIExtensionData(RenderFrame* render_frame)
: RenderFrameObserver(render_frame),
RenderFrameObserverTracker<WebUIExtensionData>(render_frame) {}

WebUIExtensionData::~WebUIExtensionData() {
}
Expand All @@ -27,7 +26,7 @@ std::string WebUIExtensionData::GetValue(const std::string& key) const {
bool WebUIExtensionData::OnMessageReceived(const IPC::Message& message) {
bool handled = true;
IPC_BEGIN_MESSAGE_MAP(WebUIExtensionData, message)
IPC_MESSAGE_HANDLER(ViewMsg_SetWebUIProperty, OnSetWebUIProperty)
IPC_MESSAGE_HANDLER(FrameMsg_SetWebUIProperty, OnSetWebUIProperty)
IPC_MESSAGE_UNHANDLED(handled = false)
IPC_END_MESSAGE_MAP()
return handled;
Expand Down
12 changes: 6 additions & 6 deletions content/renderer/web_ui_extension_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,24 @@
#include <string>

#include "base/macros.h"
#include "content/public/renderer/render_view_observer.h"
#include "content/public/renderer/render_view_observer_tracker.h"
#include "content/public/renderer/render_frame_observer.h"
#include "content/public/renderer/render_frame_observer_tracker.h"

namespace content {

class WebUIExtensionData
: public RenderViewObserver,
public RenderViewObserverTracker<WebUIExtensionData> {
: public RenderFrameObserver,
public RenderFrameObserverTracker<WebUIExtensionData> {
public:
explicit WebUIExtensionData(RenderView* render_view);
explicit WebUIExtensionData(RenderFrame* render_frame);
~WebUIExtensionData() override;

// Returns value for a given |key|. Will return an empty string if no such key
// exists in the |variable_map_|.
std::string GetValue(const std::string& key) const;

private:
// RenderViewObserver implementation.
// RenderFrameObserver implementation.
bool OnMessageReceived(const IPC::Message& message) override;
void OnDestruct() override;

Expand Down
6 changes: 2 additions & 4 deletions ui/web_dialogs/web_dialog_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,13 @@ void WebDialogUIBase::HandleRenderFrameCreated(
delegate->GetWebUIMessageHandlers(&handlers);
}

content::RenderViewHost* render_view_host =
render_frame_host->GetRenderViewHost();
if (0 != (web_ui_->GetBindings() & content::BINDINGS_POLICY_WEB_UI))
render_view_host->SetWebUIProperty("dialogArguments", dialog_args);
render_frame_host->SetWebUIProperty("dialogArguments", dialog_args);
for (WebUIMessageHandler* handler : handlers)
web_ui_->AddMessageHandler(base::WrapUnique(handler));

if (delegate)
delegate->OnDialogShown(web_ui_, render_view_host);
delegate->OnDialogShown(web_ui_, render_frame_host->GetRenderViewHost());
}

void WebDialogUIBase::OnDialogClosed(const base::ListValue* args) {
Expand Down

0 comments on commit dce1a62

Please sign in to comment.