Commit 90c20c39 authored by Alexander Timin's avatar Alexander Timin Committed by Commit Bot

Introduce WebContentsObserverList to encapsulate common behaviour

Add a dedicated helper struct for holding WebContentsObservers. This
allows us to ensure that iteration over all observers can be detected:

- This can replace existing is_notifying_observer_ WebContentsImpl
member and ensure that it's set for almost all observer iterations.

- This allows us to emit a trace event covering observer iteration.

Escape hatch is kept for now for the three places which want to return
value as soon as matching observer is found.

Also WeakPtr-based logic is added to RenderViewTerminated as some observers
are actually trying to delete WebContents there.

R=nasko@chromium.org

Change-Id: Ic94cf9bbc83b2d9bfe93a3090a4f4af3fd8f60dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2343269Reviewed-by: default avatarSami Kyöstilä <skyostil@chromium.org>
Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Commit-Queue: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797266}
parent 56b9021d
...@@ -185,6 +185,7 @@ ...@@ -185,6 +185,7 @@
X(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler")) \ X(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler")) \
X(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler.frames")) \ X(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler.frames")) \
X(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler.now")) \ X(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler.now")) \
X(TRACE_DISABLED_BY_DEFAULT("content.verbose")) \
X(TRACE_DISABLED_BY_DEFAULT("cpu_profiler")) \ X(TRACE_DISABLED_BY_DEFAULT("cpu_profiler")) \
X(TRACE_DISABLED_BY_DEFAULT("cpu_profiler.debug")) \ X(TRACE_DISABLED_BY_DEFAULT("cpu_profiler.debug")) \
X(TRACE_DISABLED_BY_DEFAULT("devtools.screenshot")) \ X(TRACE_DISABLED_BY_DEFAULT("devtools.screenshot")) \
......
...@@ -612,6 +612,7 @@ void RenderViewHostImpl::RenderProcessExited( ...@@ -612,6 +612,7 @@ void RenderViewHostImpl::RenderProcessExited(
GetWidget()->RendererExited(); GetWidget()->RendererExited();
delegate_->RenderViewTerminated(this, info.status, info.exit_code); delegate_->RenderViewTerminated(this, info.status, info.exit_code);
// |this| might have been deleted. Do not add code here.
} }
bool RenderViewHostImpl::Send(IPC::Message* msg) { bool RenderViewHostImpl::Send(IPC::Message* msg) {
......
...@@ -1395,6 +1395,40 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents, ...@@ -1395,6 +1395,40 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
WebContentsImpl* focused_web_contents_; WebContentsImpl* focused_web_contents_;
}; };
// Container for WebContentsObservers, which knows when we are iterating over
// observer set.
class WebContentsObserverList {
public:
WebContentsObserverList();
~WebContentsObserverList();
void AddObserver(WebContentsObserver* observer);
void RemoveObserver(WebContentsObserver* observer);
template <class ForEachCallable>
void ForEachObserver(const ForEachCallable& callable) {
TRACE_EVENT0("content", "Iterating over WebContentsObservers");
base::AutoReset<bool> scope(&is_notifying_observers_, true);
for (WebContentsObserver& observer : observers_) {
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("content.verbose"),
"Dispatching WebContentsObserver callback");
callable(&observer);
}
}
bool is_notifying_observers() { return is_notifying_observers_; }
// Exposed to deal with IPC message handlers which need to stop iteration
// early.
const base::ObserverList<WebContentsObserver>::Unchecked& observer_list() {
return observers_;
}
private:
bool is_notifying_observers_ = false;
base::ObserverList<WebContentsObserver>::Unchecked observers_;
};
// See WebContents::Create for a description of these parameters. // See WebContents::Create for a description of these parameters.
explicit WebContentsImpl(BrowserContext* browser_context); explicit WebContentsImpl(BrowserContext* browser_context);
...@@ -1719,7 +1753,7 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents, ...@@ -1719,7 +1753,7 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
// This MUST be listed above frame_tree_ since at destruction time the // This MUST be listed above frame_tree_ since at destruction time the
// latter might cause RenderViewHost's destructor to call us and we might use // latter might cause RenderViewHost's destructor to call us and we might use
// the observer list then. // the observer list then.
base::ObserverList<WebContentsObserver>::Unchecked observers_; WebContentsObserverList observers_;
// Associated interface receiver sets attached to this WebContents. // Associated interface receiver sets attached to this WebContents.
std::map<std::string, WebContentsReceiverSet*> receiver_sets_; std::map<std::string, WebContentsReceiverSet*> receiver_sets_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment