Skip to content

Commit

Permalink
Add WebLocalFrameObserver to replace RenderFrameObserver methods
Browse files Browse the repository at this point in the history
This allows us to reduce the size of RenderFrameImpl so that it
is not just a springboard for events to be rebroadcast on
RenderFrameObserver.

Move WillSendSubmitEvent over to this new interface.

BUG=1349894

Change-Id: I309992ab8d50d55f4c4dcadb8a37561766e8701e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3864268
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1042084}
  • Loading branch information
dtapuska authored and Chromium LUCI CQ committed Sep 1, 2022
1 parent 4f8e118 commit d5a0b00
Show file tree
Hide file tree
Showing 14 changed files with 145 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,7 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {

void SaveAndSubmitForm(const WebFormElement& form_element) {
FormTracker* tracker = autofill_agent_->form_tracker_for_testing();
static_cast<content::RenderFrameObserver*>(tracker)->WillSendSubmitEvent(
static_cast<blink::WebLocalFrameObserver*>(tracker)->WillSendSubmitEvent(
form_element);
static_cast<content::RenderFrameObserver*>(tracker)->WillSubmitForm(
form_element);
Expand Down
8 changes: 7 additions & 1 deletion components/autofill/content/renderer/form_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ namespace autofill {
using mojom::SubmissionSource;

FormTracker::FormTracker(content::RenderFrame* render_frame)
: content::RenderFrameObserver(render_frame) {
: content::RenderFrameObserver(render_frame),
blink::WebLocalFrameObserver(render_frame->GetWebFrame()) {
DCHECK_CALLED_ON_VALID_SEQUENCE(form_tracker_sequence_checker_);
}

Expand Down Expand Up @@ -214,6 +215,11 @@ void FormTracker::OnDestruct() {
ResetLastInteractedElements();
}

void FormTracker::OnFrameDetached() {
DCHECK_CALLED_ON_VALID_SEQUENCE(form_tracker_sequence_checker_);
ResetLastInteractedElements();
}

void FormTracker::FireFormSubmitted(const blink::WebFormElement& form) {
for (auto& observer : observers_)
observer.OnFormSubmitted(form);
Expand Down
9 changes: 7 additions & 2 deletions components/autofill/content/renderer/form_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "components/autofill/core/common/mojom/autofill_types.mojom.h"
#include "content/public/renderer/render_frame_observer.h"
#include "third_party/blink/public/web/web_input_element.h"
#include "third_party/blink/public/web/web_local_frame_observer.h"

namespace blink {
class WebFormElementObserver;
Expand All @@ -22,7 +23,8 @@ namespace autofill {
// TODO(crbug.com/785531): Track the select and checkbox change.
// This class is used to track user's change of form or WebFormControlElement,
// notifies observers of form's change and submission.
class FormTracker : public content::RenderFrameObserver {
class FormTracker : public content::RenderFrameObserver,
public blink::WebLocalFrameObserver {
public:
// The interface implemented by observer to get notification of form's change
// and submission.
Expand Down Expand Up @@ -104,10 +106,13 @@ class FormTracker : public content::RenderFrameObserver {
const GURL& url,
absl::optional<blink::WebNavigationType> navigation_type) override;
void WillDetach() override;
void WillSendSubmitEvent(const blink::WebFormElement& form) override;
void WillSubmitForm(const blink::WebFormElement& form) override;
void OnDestruct() override;

// content::WebLocalFrameObserver:
void OnFrameDetached() override;
void WillSendSubmitEvent(const blink::WebFormElement& form) override;

// Called in a posted task by textFieldDidChange() to work-around a WebKit bug
// http://bugs.webkit.org/show_bug.cgi?id=16976 , we also don't want to
// process element while it is changing.
Expand Down
1 change: 0 additions & 1 deletion content/public/renderer/render_frame_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ class CONTENT_EXPORT RenderFrameObserver : public IPC::Listener,
int32_t world_id) {}
virtual void DidClearWindowObject() {}
virtual void DidChangeScrollOffset() {}
virtual void WillSendSubmitEvent(const blink::WebFormElement& form) {}
virtual void WillSubmitForm(const blink::WebFormElement& form) {}
virtual void DidMatchCSS(
const blink::WebVector<blink::WebString>& newly_matching_selectors,
Expand Down
5 changes: 0 additions & 5 deletions content/renderer/render_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3612,11 +3612,6 @@ void RenderFrameImpl::DidAddMessageToConsole(
}
}

void RenderFrameImpl::WillSendSubmitEvent(const blink::WebFormElement& form) {
for (auto& observer : observers_)
observer.WillSendSubmitEvent(form);
}

void RenderFrameImpl::DidCreateDocumentLoader(
blink::WebDocumentLoader* document_loader) {
DocumentState* document_state =
Expand Down
1 change: 0 additions & 1 deletion content/renderer/render_frame_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,6 @@ class CONTENT_EXPORT RenderFrameImpl
unsigned source_line,
const blink::WebString& stack_trace) override;
void BeginNavigation(std::unique_ptr<blink::WebNavigationInfo> info) override;
void WillSendSubmitEvent(const blink::WebFormElement& form) override;
void DidCreateDocumentLoader(
blink::WebDocumentLoader* document_loader) override;
bool SwapIn(blink::WebFrame* previous_web_frame) override;
Expand Down
1 change: 1 addition & 0 deletions third_party/blink/public/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ source_set("blink_headers") {
"web/web_lifecycle_update.h",
"web/web_local_frame.h",
"web/web_local_frame_client.h",
"web/web_local_frame_observer.h",
"web/web_manifest_manager.h",
"web/web_meaningful_layout.h",
"web/web_media_inspector.h",
Expand Down
5 changes: 0 additions & 5 deletions third_party/blink/public/web/web_local_frame_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@
#include "third_party/blink/public/platform/web_worker_fetch_context.h"
#include "third_party/blink/public/web/web_ax_object.h"
#include "third_party/blink/public/web/web_document_loader.h"
#include "third_party/blink/public/web/web_form_element.h"
#include "third_party/blink/public/web/web_frame.h"
#include "third_party/blink/public/web/web_frame_load_type.h"
#include "third_party/blink/public/web/web_frame_owner_properties.h"
Expand Down Expand Up @@ -329,10 +328,6 @@ class BLINK_EXPORT WebLocalFrameClient {
virtual void DidStartLoading() {}
virtual void DidStopLoading() {}

// A form submission has been requested, but the page's submit event handler
// hasn't yet had a chance to run (and possibly alter/interrupt the submit.)
virtual void WillSendSubmitEvent(const WebFormElement&) {}

// A datasource has been created for a new navigation. The given
// datasource will become the provisional datasource for the frame.
virtual void DidCreateDocumentLoader(WebDocumentLoader*) {}
Expand Down
53 changes: 53 additions & 0 deletions third_party/blink/public/web/web_local_frame_observer.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright 2022 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef THIRD_PARTY_BLINK_PUBLIC_WEB_WEB_LOCAL_FRAME_OBSERVER_H_
#define THIRD_PARTY_BLINK_PUBLIC_WEB_WEB_LOCAL_FRAME_OBSERVER_H_

#include "base/observer_list_types.h"
#include "third_party/blink/public/platform/web_common.h"
#include "third_party/blink/public/platform/web_private_ptr.h"

namespace blink {
class WebFormElement;
class WebLocalFrame;
class WebLocalFrameImpl;

// Base class for objects that want to get notified of changes to the local
// frame.
class BLINK_EXPORT WebLocalFrameObserver : public base::CheckedObserver {
public:
// A subclass can use this to delete itself.
virtual void OnFrameDetached() = 0;

// A form submission has been requested, but the page's submit event handler
// hasn't yet had a chance to run (and possibly alter/interrupt the submit.)
virtual void WillSendSubmitEvent(const WebFormElement&) {}

// Retrieves the WebLocalFrame that is being observed. Can be null.
WebLocalFrame* GetWebLocalFrame() const;

protected:
friend class WebLocalFrameImpl;
explicit WebLocalFrameObserver(WebLocalFrame* web_local_frame);
~WebLocalFrameObserver() override;

private:
// Called when `web_local_frame_` was detached.
void WebLocalFrameDetached();

// Sets `WebLocalFrame` to track.
// Removes itself of previous (if any) `web_local_frame_` observer list and
// adds to the new `web_local_frame`.
void Observe(WebLocalFrameImpl* web_local_frame);

WebPrivatePtr<WebLocalFrameImpl,
kWebPrivatePtrDestructionSameThread,
WebPrivatePtrStrength::kWeak>
web_local_frame_;
};

} // namespace blink

#endif // THIRD_PARTY_BLINK_PUBLIC_WEB_WEB_VIEW_OBSERVER_H_
3 changes: 2 additions & 1 deletion third_party/blink/renderer/core/frame/build.gni
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,11 @@ blink_core_sources_frame = [
"web_frame.cc",
"web_frame_widget_impl.cc",
"web_frame_widget_impl.h",
"web_local_frame_client.cc",
"web_local_frame_impl.cc",
"web_local_frame_impl.h",
"web_local_frame_observer.cc",
"web_remote_frame_impl.cc",
"web_local_frame_client.cc",
"web_remote_frame_impl.h",
"window_event_handlers.h",
"window_or_worker_global_scope.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -662,8 +662,7 @@ void LocalFrameClientImpl::BeginNavigation(
}

void LocalFrameClientImpl::DispatchWillSendSubmitEvent(HTMLFormElement* form) {
if (web_frame_->Client())
web_frame_->Client()->WillSendSubmitEvent(WebFormElement(form));
web_frame_->WillSendSubmitEvent(WebFormElement(form));
}

void LocalFrameClientImpl::DidStartLoading() {
Expand Down
18 changes: 18 additions & 0 deletions third_party/blink/renderer/core/frame/web_local_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2698,6 +2698,9 @@ void WebLocalFrameImpl::WillBeDetached() {
find_in_page_->Dispose();
if (print_client_)
print_client_->WillBeDestroyed();

for (auto& observer : observers_)
observer.WebLocalFrameDetached();
}

void WebLocalFrameImpl::WillDetachParent() {
Expand Down Expand Up @@ -3091,4 +3094,19 @@ void WebLocalFrameImpl::ResetHasScrolledFocusedEditableIntoView() {
has_scrolled_focused_editable_node_into_rect_ = false;
}

void WebLocalFrameImpl::AddObserver(WebLocalFrameObserver* observer) {
// Ensure that the frame is attached.
DCHECK(GetFrame());
observers_.AddObserver(observer);
}

void WebLocalFrameImpl::RemoveObserver(WebLocalFrameObserver* observer) {
observers_.RemoveObserver(observer);
}

void WebLocalFrameImpl::WillSendSubmitEvent(const WebFormElement& form) {
for (auto& observer : observers_)
observer.WillSendSubmitEvent(form);
}

} // namespace blink
11 changes: 11 additions & 0 deletions third_party/blink/renderer/core/frame/web_local_frame_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <utility>

#include "base/dcheck_is_on.h"
#include "base/observer_list.h"
#include "base/task/single_thread_task_runner.h"
#include "base/types/pass_key.h"
#include "build/build_config.h"
Expand All @@ -59,6 +60,7 @@
#include "third_party/blink/public/platform/web_file_system_type.h"
#include "third_party/blink/public/web/web_history_commit_type.h"
#include "third_party/blink/public/web/web_local_frame.h"
#include "third_party/blink/public/web/web_local_frame_observer.h"
#include "third_party/blink/public/web/web_navigation_control.h"
#include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/core/editing/forward.h"
Expand Down Expand Up @@ -531,6 +533,12 @@ class CORE_EXPORT WebLocalFrameImpl final

virtual void Trace(Visitor*) const;

// Functions to add and remove observers for this object.
void AddObserver(WebLocalFrameObserver* observer);
void RemoveObserver(WebLocalFrameObserver* observer);

void WillSendSubmitEvent(const WebFormElement& form);

protected:
// WebLocalFrame protected overrides:
void AddMessageToConsoleImpl(const WebConsoleMessage&,
Expand Down Expand Up @@ -658,6 +666,9 @@ class CORE_EXPORT WebLocalFrameImpl final
bool has_scrolled_focused_editable_node_into_rect_ = false;

WebHistoryItem current_history_item_;

// All the registered observers.
base::ObserverList<WebLocalFrameObserver, true> observers_;
};

template <>
Expand Down
44 changes: 44 additions & 0 deletions third_party/blink/renderer/core/frame/web_local_frame_observer.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright 2022 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "third_party/blink/public/web/web_local_frame_observer.h"

#include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
#include "third_party/blink/renderer/platform/wtf/casting.h"

namespace blink {

WebLocalFrameObserver::WebLocalFrameObserver(WebLocalFrame* web_local_frame)
: web_local_frame_(To<WebLocalFrameImpl>(web_local_frame)) {
// |web_local_frame_| can be null on unit testing or if Observe() is used.
if (web_local_frame_) {
web_local_frame_->AddObserver(this);
}
}

WebLocalFrameObserver::~WebLocalFrameObserver() {
Observe(nullptr);
}

WebLocalFrame* WebLocalFrameObserver::GetWebLocalFrame() const {
return web_local_frame_.Get();
}

void WebLocalFrameObserver::Observe(WebLocalFrameImpl* web_local_frame) {
if (web_local_frame_) {
web_local_frame_->RemoveObserver(this);
}

web_local_frame_ = web_local_frame;
if (web_local_frame) {
web_local_frame->AddObserver(this);
}
}

void WebLocalFrameObserver::WebLocalFrameDetached() {
Observe(nullptr);
OnFrameDetached();
}

} // namespace blink

0 comments on commit d5a0b00

Please sign in to comment.