Removing ContentViewCore dependencies from few functions which acts as direct wrapper to WebContents

content/browser/android/content_view_core_impl.cc

Index: content/browser/android/content_view_core_impl.cc
diff --git a/content/browser/android/content_view_core_impl.cc b/content/browser/android/content_view_core_impl.cc
index 82d79ab8a83c013bb934cf89f4da2c6535d9f1cd..4daa20185e1930876bbc169221de10bf09ae9cc6 100644
--- a/content/browser/android/content_view_core_impl.cc
+++ b/content/browser/android/content_view_core_impl.cc
@@ -819,25 +819,6 @@ scoped_refptr<cc::Layer> ContentViewCoreImpl::GetLayer() const {
 // Methods called from Java via JNI
 // ----------------------------------------------------------------------------
 
-void ContentViewCoreImpl::SelectPopupMenuItems(JNIEnv* env, jobject obj,

[Yaron, 2014/08/01 16:21:12]

Sorry but it's unclear why this is moving.

[AKVT, 2014/08/01 17:09:58]

I thought to untouch only RWHVA functions. This function where shall we migrate
? Also one more doubt I have is who will own dpi_scale and other related members
? Please share your view.

[Ted C, 2014/08/01 17:26:20]

The goal of the java WebContents class is to act as a simple bridge to the
native implementation of those methods.  Ideally, the methods on the java side
would have a one to one mapping with those in web_contents.cc.  As this is
simply using the web_contents as a means for getting the RenderViewHost, it does
not belong in the WebContents class.

In cases like this, it might be more appropriate to extract the popup methods
out to a completely separate class that handles all of the interactions and
makes them a single isolated unit that doesn't get muddled with the rest of the
calls.  Granted, that may not be possible due to the message routing etc, but it
would certainly be nice to start building smaller components instead of
monolithic classes.

[AKVT, 2014/08/02 10:03:59]

Thanks Ted. I have removed this function from the patch to take care of only
direct dependent WebContents functions. As discussed will take care of RWHVA
functions to new class and remaining independent functions I will club into one
Class for better restructuring.

-                                               jintArray indices) {
-  RenderViewHostImpl* rvhi = static_cast<RenderViewHostImpl*>(
-      web_contents_->GetRenderViewHost());
-  DCHECK(rvhi);
-  if (indices == NULL) {
-    rvhi->DidCancelPopupMenu();
-    return;
-  }
-
-  int selected_count = env->GetArrayLength(indices);
-  std::vector<int> selected_indices;
-  jint* indices_ptr = env->GetIntArrayElements(indices, NULL);
-  for (int i = 0; i < selected_count; ++i)
-    selected_indices.push_back(indices_ptr[i]);
-  env->ReleaseIntArrayElements(indices, indices_ptr, JNI_ABORT);
-  rvhi->DidSelectPopupMenuItems(selected_indices);
-}
-
 void ContentViewCoreImpl::LoadUrl(
     JNIEnv* env, jobject obj,
     jstring url,
@@ -895,15 +876,6 @@ void ContentViewCoreImpl::LoadUrl(
   LoadUrl(params);
 }
 
-ScopedJavaLocalRef<jstring> ContentViewCoreImpl::GetURL(
-    JNIEnv* env, jobject) const {
-  return ConvertUTF8ToJavaString(env, GetWebContents()->GetURL().spec());
-}
-
-jboolean ContentViewCoreImpl::IsIncognito(JNIEnv* env, jobject obj) {
-  return GetWebContents()->GetBrowserContext()->IsOffTheRecord();
-}
-
 WebContents* ContentViewCoreImpl::GetWebContents() const {
   return web_contents_;
 }
@@ -1499,28 +1471,6 @@ void ContentViewCoreImpl::ExtractSmartClipData(JNIEnv* env,
       GetWebContents()->GetRoutingID(), rect));
 }
 
-void ContentViewCoreImpl::ResumeResponseDeferredAtStart(JNIEnv* env,
-                                                        jobject obj) {
-  static_cast<WebContentsImpl*>(GetWebContents())->
-      ResumeResponseDeferredAtStart();
-}
-
-void ContentViewCoreImpl::SetHasPendingNavigationTransitionForTesting(
-    JNIEnv* env,
-    jobject obj) {
-  RenderFrameHost* frame = static_cast<WebContentsImpl*>(GetWebContents())->
-      GetMainFrame();
-  BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
-      base::Bind(
-          &TransitionRequestManager::SetHasPendingTransitionRequest,
-          base::Unretained(TransitionRequestManager::GetInstance()),
-          frame->GetProcess()->GetID(),
-          frame->GetRoutingID(),
-          true));
-}
-
 jint ContentViewCoreImpl::GetCurrentRenderProcessId(JNIEnv* env, jobject obj) {
   return GetRenderProcessIdFromRenderViewHost(
       web_contents_->GetRenderViewHost());