Resolves layering violation in SynchronousCompositorHost creation
SynchronousCompositorHost created in RWHVA has a reference to
WebContents, which violates the hierarchical access rule. This
CL resolves it by having WebContentsViewAndroid keep
SynchronousCompositorClient instance and pass it to RWHVA.
BUG=626765
Committed: https://crrev.com/8f60271614652b6110081128604470460af5dadb
Review-Url: https://codereview.chromium.org/2487713002
Cr-Original-Commit-Position: refs/heads/master@{#431383}
Cr-Commit-Position: refs/heads/master@{#431565}
diff --git a/content/browser/android/synchronous_compositor_host.cc b/content/browser/android/synchronous_compositor_host.cc
index c0d33fb..4f2e1f07 100644
--- a/content/browser/android/synchronous_compositor_host.cc
+++ b/content/browser/android/synchronous_compositor_host.cc
@@ -13,7 +13,6 @@
#include "base/trace_event/trace_event_argument.h"
#include "content/browser/android/synchronous_compositor_browser_filter.h"
#include "content/browser/renderer_host/render_widget_host_view_android.h"
-#include "content/browser/web_contents/web_contents_android.h"
#include "content/browser/web_contents/web_contents_impl.h"
#include "content/common/android/sync_compositor_messages.h"
#include "content/common/android/sync_compositor_statics.h"
@@ -32,41 +31,23 @@
namespace content {
// static
-void SynchronousCompositor::SetClientForWebContents(
- WebContents* contents,
- SynchronousCompositorClient* client) {
- DCHECK(contents);
- DCHECK(client);
- WebContentsAndroid* web_contents_android =
- static_cast<WebContentsImpl*>(contents)->GetWebContentsAndroid();
- DCHECK(!web_contents_android->synchronous_compositor_client());
- web_contents_android->set_synchronous_compositor_client(client);
-}
-
-// static
std::unique_ptr<SynchronousCompositorHost> SynchronousCompositorHost::Create(
- RenderWidgetHostViewAndroid* rwhva,
- WebContents* web_contents) {
- DCHECK(web_contents);
- WebContentsAndroid* web_contents_android =
- static_cast<WebContentsImpl*>(web_contents)->GetWebContentsAndroid();
- if (!web_contents_android->synchronous_compositor_client())
+ RenderWidgetHostViewAndroid* rwhva) {
+ if (!rwhva->synchronous_compositor_client())
return nullptr; // Not using sync compositing.
base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
bool use_in_proc_software_draw =
command_line->HasSwitch(switches::kSingleProcess);
return base::WrapUnique(new SynchronousCompositorHost(
- rwhva, web_contents_android->synchronous_compositor_client(),
- use_in_proc_software_draw));
+ rwhva, use_in_proc_software_draw));
}
SynchronousCompositorHost::SynchronousCompositorHost(
RenderWidgetHostViewAndroid* rwhva,
- SynchronousCompositorClient* client,
bool use_in_proc_software_draw)
: rwhva_(rwhva),
- client_(client),
+ client_(rwhva->synchronous_compositor_client()),
ui_task_runner_(BrowserThread::GetTaskRunnerForThread(BrowserThread::UI)),
process_id_(rwhva_->GetRenderWidgetHost()->GetProcess()->GetID()),
routing_id_(rwhva_->GetRenderWidgetHost()->GetRoutingID()),
diff --git a/content/browser/android/synchronous_compositor_host.h b/content/browser/android/synchronous_compositor_host.h
index 0260083..9d232c5c 100644
--- a/content/browser/android/synchronous_compositor_host.h
+++ b/content/browser/android/synchronous_compositor_host.h
@@ -41,8 +41,7 @@
class SynchronousCompositorHost : public SynchronousCompositor {
public:
static std::unique_ptr<SynchronousCompositorHost> Create(
- RenderWidgetHostViewAndroid* rwhva,
- WebContents* web_contents);
+ RenderWidgetHostViewAndroid* rwhva);
~SynchronousCompositorHost() override;
@@ -82,7 +81,6 @@
friend class SynchronousCompositorBase;
SynchronousCompositorHost(RenderWidgetHostViewAndroid* rwhva,
- SynchronousCompositorClient* client,
bool use_in_proc_software_draw);
void CompositorFrameSinkCreated();
bool DemandDrawSwInProc(SkCanvas* canvas);
diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc
index 80c0830..59d11e0f 100644
--- a/content/browser/renderer_host/render_widget_host_view_android.cc
+++ b/content/browser/renderer_host/render_widget_host_view_android.cc
@@ -450,6 +450,7 @@
this),
stylus_text_selector_(this),
using_browser_compositor_(CompositorImpl::IsInitialized()),
+ synchronous_compositor_client_(nullptr),
frame_evictor_(new DelegatedFrameEvictor(this)),
locks_on_frame_count_(0),
observing_root_window_(false),
@@ -1193,6 +1194,14 @@
}
}
+void RenderWidgetHostViewAndroid::SetSynchronousCompositorClient(
+ SynchronousCompositorClient* client) {
+ synchronous_compositor_client_ = client;
+ if (!sync_compositor_ && synchronous_compositor_client_) {
+ sync_compositor_ = SynchronousCompositorHost::Create(this);
+ }
+}
+
bool RenderWidgetHostViewAndroid::SupportsAnimation() const {
// The synchronous (WebView) compositor does not have a proper browser
// compositor with which to drive animations.
@@ -1776,11 +1785,6 @@
overscroll_controller_ = CreateOverscrollController(
content_view_core_, ui::GetScaleFactorForNativeView(GetNativeView()));
}
-
- if (!sync_compositor_) {
- sync_compositor_ = SynchronousCompositorHost::Create(
- this, content_view_core_->GetWebContents());
- }
}
void RenderWidgetHostViewAndroid::RunAckCallbacks() {
diff --git a/content/browser/renderer_host/render_widget_host_view_android.h b/content/browser/renderer_host/render_widget_host_view_android.h
index 10f335f..e8754e0 100644
--- a/content/browser/renderer_host/render_widget_host_view_android.h
+++ b/content/browser/renderer_host/render_widget_host_view_android.h
@@ -62,6 +62,7 @@
class RenderWidgetHost;
class RenderWidgetHostImpl;
class SynchronousCompositorHost;
+class SynchronousCompositorClient;
struct NativeWebKeyboardEvent;
struct TextInputState;
@@ -237,6 +238,12 @@
void SynchronousFrameMetadata(cc::CompositorFrameMetadata frame_metadata);
+ void SetSynchronousCompositorClient(SynchronousCompositorClient* client);
+
+ SynchronousCompositorClient* synchronous_compositor_client() const {
+ return synchronous_compositor_client_;
+ }
+
static void OnContextLost();
private:
@@ -351,6 +358,8 @@
const bool using_browser_compositor_;
std::unique_ptr<SynchronousCompositorHost> sync_compositor_;
+ SynchronousCompositorClient* synchronous_compositor_client_;
+
std::unique_ptr<DelegatedFrameEvictor> frame_evictor_;
size_t locks_on_frame_count_;
diff --git a/content/browser/web_contents/web_contents_android.cc b/content/browser/web_contents/web_contents_android.cc
index 05b779e5..9be39aeb 100644
--- a/content/browser/web_contents/web_contents_android.cc
+++ b/content/browser/web_contents/web_contents_android.cc
@@ -247,7 +247,6 @@
WebContentsAndroid::WebContentsAndroid(WebContentsImpl* web_contents)
: web_contents_(web_contents),
navigation_controller_(&(web_contents->GetController())),
- synchronous_compositor_client_(nullptr),
weak_factory_(this) {
g_allocated_web_contents_androids.Get().insert(this);
JNIEnv* env = AttachCurrentThread();
diff --git a/content/browser/web_contents/web_contents_android.h b/content/browser/web_contents/web_contents_android.h
index 2fb3e70..d23f93a2 100644
--- a/content/browser/web_contents/web_contents_android.h
+++ b/content/browser/web_contents/web_contents_android.h
@@ -21,7 +21,6 @@
namespace content {
-class SynchronousCompositorClient;
class WebContentsImpl;
// Android wrapper around WebContents that provides safer passage from java and
@@ -175,13 +174,6 @@
void ReloadLoFiImages(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
- void set_synchronous_compositor_client(SynchronousCompositorClient* client) {
- synchronous_compositor_client_ = client;
- }
- SynchronousCompositorClient* synchronous_compositor_client() const {
- return synchronous_compositor_client_;
- }
-
int DownloadImage(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj,
const base::android::JavaParamRef<jstring>& url,
@@ -216,7 +208,6 @@
WebContentsImpl* web_contents_;
NavigationControllerAndroid navigation_controller_;
base::android::ScopedJavaGlobalRef<jobject> obj_;
- SynchronousCompositorClient* synchronous_compositor_client_;
base::WeakPtrFactory<WebContentsAndroid> weak_factory_;
diff --git a/content/browser/web_contents/web_contents_view_android.cc b/content/browser/web_contents/web_contents_view_android.cc
index 8a55bfe3..338d07e 100644
--- a/content/browser/web_contents/web_contents_view_android.cc
+++ b/content/browser/web_contents/web_contents_view_android.cc
@@ -46,6 +46,22 @@
results->is_monochrome = display.is_monochrome();
}
+// static
+void SynchronousCompositor::SetClientForWebContents(
+ WebContents* contents,
+ SynchronousCompositorClient* client) {
+ DCHECK(contents);
+ DCHECK(client);
+ WebContentsViewAndroid* wcva = static_cast<WebContentsViewAndroid*>(
+ static_cast<WebContentsImpl*>(contents)->GetView());
+ DCHECK(!wcva->synchronous_compositor_client());
+ wcva->set_synchronous_compositor_client(client);
+ RenderWidgetHostViewAndroid* rwhv = static_cast<RenderWidgetHostViewAndroid*>(
+ contents->GetRenderWidgetHostView());
+ if (rwhv)
+ rwhv->SetSynchronousCompositorClient(client);
+}
+
WebContentsView* CreateWebContentsView(
WebContentsImpl* web_contents,
WebContentsViewDelegate* delegate,
@@ -61,7 +77,8 @@
WebContentsViewDelegate* delegate)
: web_contents_(web_contents),
content_view_core_(NULL),
- delegate_(delegate) {
+ delegate_(delegate),
+ synchronous_compositor_client_(nullptr) {
}
WebContentsViewAndroid::~WebContentsViewAndroid() {
@@ -189,7 +206,10 @@
// order to paint it. See ContentView::GetRenderWidgetHostViewAndroid for an
// example of how this is achieved for InterstitialPages.
RenderWidgetHostImpl* rwhi = RenderWidgetHostImpl::From(render_widget_host);
- return new RenderWidgetHostViewAndroid(rwhi, content_view_core_);
+ RenderWidgetHostViewAndroid* rwhv =
+ new RenderWidgetHostViewAndroid(rwhi, content_view_core_);
+ rwhv->SetSynchronousCompositorClient(synchronous_compositor_client_);
+ return rwhv;
}
RenderWidgetHostViewBase* WebContentsViewAndroid::CreateViewForPopupWidget(
diff --git a/content/browser/web_contents/web_contents_view_android.h b/content/browser/web_contents/web_contents_view_android.h
index bd2c52dc..f2ddca6 100644
--- a/content/browser/web_contents/web_contents_view_android.h
+++ b/content/browser/web_contents/web_contents_view_android.h
@@ -18,6 +18,8 @@
namespace content {
class ContentViewCoreImpl;
+class RenderWidgetHostViewAndroid;
+class SynchronousCompositorClient;
class WebContentsImpl;
// Android-specific implementation of the WebContentsView.
@@ -33,6 +35,14 @@
// by the UI frontend.
void SetContentViewCore(ContentViewCoreImpl* content_view_core);
+ void set_synchronous_compositor_client(SynchronousCompositorClient* client) {
+ synchronous_compositor_client_ = client;
+ }
+
+ SynchronousCompositorClient* synchronous_compositor_client() const {
+ return synchronous_compositor_client_;
+ }
+
// WebContentsView implementation --------------------------------------------
gfx::NativeView GetNativeView() const override;
gfx::NativeView GetContentNativeView() const override;
@@ -103,6 +113,9 @@
// The native view associated with the contents of the web.
ui::ViewAndroid view_;
+ // Interface used to get notified of events from the synchronous compositor.
+ SynchronousCompositorClient* synchronous_compositor_client_;
+
DISALLOW_COPY_AND_ASSIGN(WebContentsViewAndroid);
};