Remove GetActiveEntry usage from content.

content/browser/android/content_view_core_impl.cc

 // Copyright 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/android/content_view_core_impl.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
@@ skipping 785 matching lines @@
 jint ContentViewCoreImpl::GetCurrentRenderProcessId(JNIEnv* env, jobject obj) {
   return GetRenderProcessIdFromRenderViewHost(
       web_contents_->GetRenderViewHost());
 }
 
 ScopedJavaLocalRef<jstring> ContentViewCoreImpl::GetURL(
     JNIEnv* env, jobject) const {
   // The current users of the Java API expect to use the active entry
   // rather than the visible entry, which is exposed by WebContents::GetURL.
   content::NavigationEntry* entry =
-      web_contents_->GetController().GetActiveEntry();
+      web_contents_->GetController().GetVisibleEntry();

[Charlie Reis, 2013/08/16 22:10:14]

This goes against the comment, which Martin Kosiba and I added as part of
introducing GetVisibleEntry.  However, it was necessary at the time to pass the
testNotCalledOnBackForwardNavigation test in android_dbg_triggered_tests, which
you appear to have passed:
http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...

I wonder if the requirements have changed.  If so, I'd say stick with
GetVisibleEntry and remove the comment.  @mkosiba, can you confirm that this is
ok now?

[mkosiba (inactive), 2013/08/19 14:49:38]

Hey! Thanks for remembering about this! We use a transition type to mark
API-initiated navigations now (instead of stashing their URL) which is why the
test passes with this CL.
Changing this here will change the WebView.getUrl semantics slightly, but in
this case it should be fine. 

[nasko, 2013/09/12 17:56:31]

Done.

   GURL url = entry ? entry->GetVirtualURL() : GURL::EmptyGURL();
   return ConvertUTF8ToJavaString(env, url.spec());
 }
 
 ScopedJavaLocalRef<jstring> ContentViewCoreImpl::GetTitle(
     JNIEnv* env, jobject obj) const {
   return ConvertUTF16ToJavaString(env, GetWebContents()->GetTitle());
 }
 
 jboolean ContentViewCoreImpl::IsIncognito(JNIEnv* env, jobject obj) {
@@ skipping 607 matching lines @@
 
     AddNavigationEntryToHistory(
         env, obj, history, controller.GetEntryAtIndex(i), i);
     num_added++;
   }
 }
 
 ScopedJavaLocalRef<jstring>
 ContentViewCoreImpl::GetOriginalUrlForActiveNavigationEntry(JNIEnv* env,
                                                             jobject obj) {
-  NavigationEntry* entry = web_contents_->GetController().GetActiveEntry();
+  NavigationEntry* entry = web_contents_->GetController().GetVisibleEntry();

[mkosiba (inactive), 2013/08/19 14:49:38]

@apiccion
FYI. This should be fine but just to be extra sure we should check that
navigations from chrome://* pages don't trigger intercepting after this lands.
Could you take care of that? Want me to file a bug?

[nasko, 2013/09/12 17:56:31]

@mkosiba, @apiccion
What is the conclusion on this? Is the change fine? Do we need to file any bugs?

   if (entry == NULL)
     return ScopedJavaLocalRef<jstring>(env, NULL);
   return ConvertUTF8ToJavaString(env, entry->GetOriginalRequestURL().spec());
 }
 
 int ContentViewCoreImpl::GetNativeImeAdapter(JNIEnv* env, jobject obj) {
   RenderWidgetHostViewAndroid* rwhva = GetRenderWidgetHostViewAndroid();
   if (!rwhva)
     return 0;
   return rwhva->GetNativeImeAdapter();
@@ skipping 46 matching lines @@
       base::Bind(&JavaScriptResultCallback, j_callback);
 
   host->ExecuteJavascriptInWebFrameCallbackResult(
       string16(),  // frame_xpath
       ConvertJavaStringToUTF16(env, script),
       c_callback);
 }
 
 bool ContentViewCoreImpl::GetUseDesktopUserAgent(
     JNIEnv* env, jobject obj) {
-  NavigationEntry* entry = web_contents_->GetController().GetActiveEntry();
+  NavigationEntry* entry = web_contents_->GetController().GetVisibleEntry();
   return entry && entry->GetIsOverridingUserAgent();
 }
 
 void ContentViewCoreImpl::UpdateImeAdapter(int native_ime_adapter,
                                            int text_input_type,
                                            const std::string& text,
                                            int selection_start,
                                            int selection_end,
                                            int composition_start,
                                            int composition_end,
@@ skipping 28 matching lines @@
 
 void ContentViewCoreImpl::SetUseDesktopUserAgent(
     JNIEnv* env,
     jobject obj,
     jboolean enabled,
     jboolean reload_on_state_change) {
   if (GetUseDesktopUserAgent(env, obj) == enabled)
     return;
 
   // Make sure the navigation entry actually exists.
-  NavigationEntry* entry = web_contents_->GetController().GetActiveEntry();
+  NavigationEntry* entry = web_contents_->GetController().GetVisibleEntry();

[mkosiba (inactive), 2013/08/19 14:49:38]

@dfalcantara:
FYI. I think this should be fine for RDS since the user is toggling the setting
for whatever URL we're displaying in the address bar.

[gone, 2013/08/19 18:40:39]

Should be alright if the visible entry updates before the page has completely
loaded (in case users try to get the desktop site before the page has finished
loading).

[nasko, 2013/09/12 17:56:31]

Done.

   if (!entry)
     return;
 
   // Set the flag in the NavigationEntry.
   entry->SetIsOverridingUserAgent(enabled);
 
   // Send the override to the renderer.
   if (reload_on_state_change) {
     // Reloading the page will send the override down as part of the
     // navigation IPC message.
@@ skipping 33 matching lines @@
       reinterpret_cast<ui::ViewAndroid*>(view_android),
       reinterpret_cast<ui::WindowAndroid*>(window_android));
   return reinterpret_cast<jint>(view);
 }
 
 bool RegisterContentViewCore(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 }  // namespace content