WebContentsObserver and TabObserver update for PlzNavigate

content/browser/android/web_contents_observer_proxy.cc

Index: content/browser/android/web_contents_observer_proxy.cc
diff --git a/content/browser/android/web_contents_observer_proxy.cc b/content/browser/android/web_contents_observer_proxy.cc
index e6a4c9dd69fe76a10ee805294bb50b2b84846257..cadc5ae4b32630d3e3e2e92e45fcf60024ca8ea6 100644
--- a/content/browser/android/web_contents_observer_proxy.cc
+++ b/content/browser/android/web_contents_observer_proxy.cc
@@ -81,17 +81,6 @@ void WebContentsObserverProxy::RenderProcessGone(
   Java_WebContentsObserverProxy_renderProcessGone(env, obj, was_oom_protected);
 }
 
-void WebContentsObserverProxy::DidFinishNavigation(
-    NavigationHandle* navigation_handle) {
-  JNIEnv* env = AttachCurrentThread();
-  ScopedJavaLocalRef<jobject> obj(java_observer_);
-  ScopedJavaLocalRef<jstring> jstring_url(
-      ConvertUTF8ToJavaString(env, web_contents()->GetVisibleURL().spec()));
-  Java_WebContentsObserverProxy_didFinishNavigation(
-      env, obj, navigation_handle->IsInMainFrame(),
-      navigation_handle->IsErrorPage(), navigation_handle->HasCommitted());
-}
-
 void WebContentsObserverProxy::DidStartLoading() {
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jobject> obj(java_observer_);
@@ -219,6 +208,37 @@ void WebContentsObserverProxy::DidCommitProvisionalLoadForFrame(
       !render_frame_host->GetParent(), jstring_url, transition_type);
 }
 
+void WebContentsObserverProxy::DidStartNavigation(
+    NavigationHandle* navigation_handle) {
+  JNIEnv* env = AttachCurrentThread();
+  ScopedJavaLocalRef<jobject> obj(java_observer_);
+  ScopedJavaLocalRef<jstring> jstring_url(
+      ConvertUTF8ToJavaString(env, navigation_handle->GetURL().spec()));
+  Java_WebContentsObserverProxy_didStartNavigation(
+      env, obj, jstring_url, navigation_handle->IsInMainFrame(),
+      navigation_handle->IsErrorPage());
+}
+
+void WebContentsObserverProxy::DidFinishNavigation(
+    NavigationHandle* navigation_handle) {
+  JNIEnv* env = AttachCurrentThread();
+  ScopedJavaLocalRef<jobject> obj(java_observer_);
+  ScopedJavaLocalRef<jstring> jstring_url(
+      ConvertUTF8ToJavaString(env, navigation_handle->GetURL().spec()));
+  jboolean is_reload = false;
+
+  if (navigation_handle->HasCommitted()) {
+    is_reload = ui::PageTransitionCoreTypeIs(

[Ted C, 2017/01/06 21:50:59]

pass the page transition type directly instead of is_reload.

There is a problem where this should be an invalid value before committed.  In
that case, I would consider passing -1 to the java WebContentsObserverProxy, but
I might change the java interface to pass a capital I Integer in the param list
and convert the -1 to null in that case.

Another option would be to add a new enum in c++ of

PAGE_TRANSITION_INVALID = -1; or something.  Might be cleaner and avoid the java
checking, but then you'd need to get sign of of all those owners.  I don't know
if it is worth floating the value by them first or if we should just isolate
this to java for now.

[sky, 2017/01/06 23:14:43]

If we add INVALID it means all existing code would have to deal with INVALID,
which it doesn't know. That would be confusing to many places and likely add a
slew of DCHECKs. As we've got this far within INVALID I'm inclined not to add it
now.

+        navigation_handle->GetPageTransition(), ui::PAGE_TRANSITION_RELOAD);
+  }
+
+  Java_WebContentsObserverProxy_didFinishNavigation(
+      env, obj, jstring_url, navigation_handle->IsInMainFrame(),
+      navigation_handle->IsErrorPage(), navigation_handle->HasCommitted(),
+      navigation_handle->IsSamePage(), is_reload,
+      navigation_handle->GetNetErrorCode());
+}
+
 void WebContentsObserverProxy::DidFinishLoad(RenderFrameHost* render_frame_host,
                                              const GURL& validated_url) {
   JNIEnv* env = AttachCurrentThread();