Remove access to WebContents in RWHVA::SynchronousFrameMetadata()

content/browser/renderer_host/render_widget_host_view_android.cc

 // Copyright (c) 2012 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 "content/browser/renderer_host/render_widget_host_view_android.h"
 
 #include <android/bitmap.h>
 
 #include <utility>
 
@@ skipping 417 matching lines @@
   while (RenderWidgetHost* widget = widgets->GetNextHost()) {
     if (widget->GetView()) {
       static_cast<RenderWidgetHostViewAndroid*>(widget->GetView())
           ->OnLostResources();
     }
   }
 }
 
 RenderWidgetHostViewAndroid::RenderWidgetHostViewAndroid(
     RenderWidgetHostImpl* widget_host,
-    ContentViewCoreImpl* content_view_core)
+    ContentViewCoreImpl* content_view_core,
+    RenderWidgetHostViewAndroidDelegate* delegate)
     : host_(widget_host),
       outstanding_vsync_requests_(0),
       is_showing_(!widget_host->is_hidden()),
       is_window_visible_(true),
       is_window_activity_started_(true),
       content_view_core_(nullptr),
       ime_adapter_android_(this),
       cached_background_color_(SK_ColorWHITE),
       last_compositor_frame_sink_id_(kUndefinedCompositorFrameSinkId),
       gesture_provider_(ui::GetGestureProviderConfig(
                             ui::GestureProviderConfigType::CURRENT_PLATFORM),
                         this),
       stylus_text_selector_(this),
       using_browser_compositor_(CompositorImpl::IsInitialized()),
       frame_evictor_(new DelegatedFrameEvictor(this)),
+      delegate_(delegate),
       locks_on_frame_count_(0),
       observing_root_window_(false),
       weak_ptr_factory_(this) {
   // Set the layer which will hold the content layer for this view. The content
   // layer is managed by the DelegatedFrameHost.
   view_.SetLayer(cc::Layer::Create());
   if (using_browser_compositor_) {
     delegated_frame_host_.reset(new ui::DelegatedFrameHostAndroid(
         &view_, cached_background_color_,
         base::Bind(&RenderWidgetHostViewAndroid::ReturnResources,
@@ skipping 130 matching lines @@
 RenderWidgetHostViewAndroid::GetNativeViewAccessible() {
   NOTIMPLEMENTED();
   return NULL;
 }
 
 void RenderWidgetHostViewAndroid::Focus() {
   host_->Focus();
   if (overscroll_controller_)
     overscroll_controller_->Enable();
   if (content_view_core_) {
-    WebContentsImpl* web_contents_impl =
-        static_cast<WebContentsImpl*>(content_view_core_->GetWebContents());
-    if (web_contents_impl->ShowingInterstitialPage())
+    if (delegate_->ShowingInterstitialPage())
       content_view_core_->ForceUpdateImeAdapter(GetNativeImeAdapter());

[boliu, 2016/11/07 18:44:36]

I think this can move to WebContentsViewAndroid, maybe?

[Jinsuk Kim, 2016/11/07 23:26:36]

Done.

   }
 }
 
 bool RenderWidgetHostViewAndroid::HasFocus() const {
   if (!content_view_core_)
     return false;  // ContentViewCore not created yet.
 
   return content_view_core_->HasFocus();
 }
 
@@ skipping 566 matching lines @@
   if (host_ && host_->input_router()) {
     host_->input_router()->NotifySiteIsMobileOptimized(
         is_mobile_optimized);
   }
 
   // This is a subset of OnSwapCompositorFrame() used in the synchronous
   // compositor flow.
   OnFrameMetadataUpdated(frame_metadata.Clone(), false);
 
   // DevTools ScreenCast support for Android WebView.
-  WebContents* web_contents = content_view_core_->GetWebContents();
-  if (DevToolsAgentHost::HasFor(web_contents)) {
-    scoped_refptr<DevToolsAgentHost> dtah =
-        DevToolsAgentHost::GetOrCreateFor(web_contents);
+  scoped_refptr<DevToolsAgentHost> dtah = delegate_->GetDevToolsAgentHost();
+  if (dtah) {
     // Unblock the compositor.
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::Bind(
             &RenderFrameDevToolsAgentHost::SynchronousSwapCompositorFrame,
             static_cast<RenderFrameDevToolsAgentHost*>(dtah.get()),
             base::Passed(&frame_metadata)));
   }
 }
 
@@ skipping 576 matching lines @@
     selection_controller_ = CreateSelectionController(this, content_view_core_);
 
   if (!overscroll_controller_ &&
       view_.GetWindowAndroid()->GetCompositor()) {
     overscroll_controller_ = CreateOverscrollController(
         content_view_core_, ui::GetScaleFactorForNativeView(GetNativeView()));
   }
 
   if (!sync_compositor_) {
     sync_compositor_ = SynchronousCompositorHost::Create(
         this, content_view_core_->GetWebContents());

[boliu, 2016/11/07 18:44:36]

you missed this one. along with the DevToolsAgentHost one, these two are
specifically for synchronous compositor (ie webview), which is very different
from how chrome rendering works

So SynchronousCompositorHost is in the same layer as RWHVA, and should not
access WebContents either. There is already SynchronousCompositorClient
interface to talk up to the content embedder, perhaps there is a way to expand
that interface to delegate for content as well, but without expanding the scope
of the actual public interface, perhaps a subclass + composition pattern?

[Jinsuk Kim, 2016/11/07 23:26:36]

Modified so that SynchronousCompositorClient is passed to RWVHA - then Host can
be created without WebContents.

With just one new public method in DevToolAgentHost (which I belive is
acceptable), the other can do without WebContents too.

   }
 }
 
 void RenderWidgetHostViewAndroid::RunAckCallbacks() {
   while (!ack_callbacks_.empty()) {
     ack_callbacks_.front().Run();
     ack_callbacks_.pop();
   }
 }
 
@@ skipping 161 matching lines @@
     case ui::MotionEvent::ACTION_UP:
     case ui::MotionEvent::ACTION_POINTER_UP:
       UMA_HISTOGRAM_CUSTOM_COUNTS("Event.Latency.OS.TOUCH_RELEASED",
                                   delta.InMicroseconds(), 1, 1000000, 50);
     default:
       return;
   }
 }
 
 }  // namespace content