weblayer: Implement TabCallback.onTitleUpdated

There are two options to implement a callback for title changes.
* Implement callback for when the active nav entry changes, and a
  callback for when the title of the nav entry changes
* A callback for when the title of a tab changes, ignoring details about
  things like navigations.

This CL takes the second option, since it's both simpler to implement
and simpler to use.

Bug: 1060997
Change-Id: I5ed1a3e41abcf5f7d7cf3c65cbd94533e8d9c36f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115695
Commit-Queue: Bo <boliu@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753490}

weblayer/browser/android/javatests/src/org/chromium/weblayer/test/TabCallbackTest.java

@@ -293,4 +293,36 @@ public class TabCallbackTest {
         callbackHelper.waitForCallback(callCount);
         Assert.assertEquals(false, isTabModalShowingResult[0]);
     }
+
+    @Test
+    @SmallTest
+    public void testOnTitleUpdated() throws TimeoutException {
+        String startupUrl = "about:blank";
+        InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl(startupUrl);
+
+        String titles[] = new String[1];
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            Tab tab = activity.getTab();
+            TabCallback callback = new TabCallback() {
+                @Override
+                public void onTitleUpdated(String title) {
+                    titles[0] = title;
+                }
+            };
+            tab.registerTabCallback(callback);
+        });
+
+        String url = mActivityTestRule.getTestDataURL("simple_page.html");
+        mActivityTestRule.navigateAndWait(url);
+        // Use polling because title is allowed to go through multiple transitions.
+        CriteriaHelper.pollUiThread(() -> { return "OK".equals(titles[0]); });
+
+        url = mActivityTestRule.getTestDataURL("shakespeare.html");
+        mActivityTestRule.navigateAndWait(url);
+        CriteriaHelper.pollUiThread(() -> { return titles[0].endsWith("shakespeare.html"); });
+
+        mActivityTestRule.executeScriptSync("document.title = \"foobar\";", false);
+        Assert.assertEquals("foobar", titles[0]);
+        CriteriaHelper.pollUiThread(() -> { return "foobar".equals(titles[0]); });
+    }
 }

weblayer/browser/java/org/chromium/weblayer_private/TabCallbackProxy.java

@@ -10,6 +10,7 @@ import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.NativeMethods;
 import org.chromium.weblayer_private.interfaces.ITabClient;
+import org.chromium.weblayer_private.interfaces.ObjectWrapper;
 
 /**
  * Owns the C++ TabCallbackProxy class, which is responsible for forwarding all
@@ -41,6 +42,12 @@ public final class TabCallbackProxy {
         mClient.onRenderProcessGone();
     }
 
+    @CalledByNative
+    private void onTitleUpdated(String title) throws RemoteException {
+        if (WebLayerFactoryImpl.getClientMajorVersion() < 83) return;
+        mClient.onTitleUpdated(ObjectWrapper.wrap(title));
+    }
+
     @NativeMethods
     interface Natives {
         long createTabCallbackProxy(TabCallbackProxy proxy, long tab);

weblayer/browser/java/org/chromium/weblayer_private/interfaces/ITabClient.aidl

@@ -26,4 +26,7 @@ interface ITabClient {
 
   // Added in M82.
   void onTabModalStateChanged(in boolean isTabModalShowing) = 5;
+
+  // Added in M83.
+  void onTitleUpdated(in IObjectWrapper title) = 6;
 }

weblayer/browser/tab_callback_proxy.cc

@@ -39,6 +39,12 @@ void TabCallbackProxy::OnRenderProcessGone() {
                                             java_observer_);
 }
 
+void TabCallbackProxy::OnTitleUpdated(const base::string16& title) {
+  JNIEnv* env = AttachCurrentThread();
+  Java_TabCallbackProxy_onTitleUpdated(
+      env, java_observer_, base::android::ConvertUTF16ToJavaString(env, title));
+}
+
 static jlong JNI_TabCallbackProxy_CreateTabCallbackProxy(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& proxy,

weblayer/browser/tab_callback_proxy.h

@@ -25,6 +25,7 @@ class TabCallbackProxy : public TabObserver {
   // TabObserver:
   void DisplayedUrlChanged(const GURL& url) override;
   void OnRenderProcessGone() override;
+  void OnTitleUpdated(const base::string16& title) override;
 
  private:
   Tab* tab_;

weblayer/browser/tab_impl.cc

@@ -231,6 +231,7 @@ TabImpl::~TabImpl() {
   // partially destructed. DidFinishNavigation can be called while destroying
   // WebContents, so stop observing first.
   Observe(nullptr);
+  web_contents_->SetDelegate(nullptr);
   web_contents_.reset();
 }
 
@@ -450,11 +451,26 @@ content::WebContents* TabImpl::OpenURLFromTab(
 
 void TabImpl::NavigationStateChanged(content::WebContents* source,
                                      content::InvalidateTypes changed_flags) {
+  DCHECK_EQ(web_contents_.get(), source);
   if (changed_flags & content::INVALIDATE_TYPE_URL) {
     for (auto& observer : observers_)
       observer.DisplayedUrlChanged(source->GetVisibleURL());
     UpdateBrowserVisibleSecurityStateIfNecessary();
   }
+
+  // TODO(crbug.com/1064582): INVALIDATE_TYPE_TITLE is called only when a title
+  // is set on the active navigation entry, but not when the active entry
+  // changes, so check INVALIDATE_TYPE_LOAD here as well. However this should
+  // be fixed and INVALIDATE_TYPE_LOAD should be removed.
+  if (changed_flags &
+      (content::INVALIDATE_TYPE_TITLE | content::INVALIDATE_TYPE_LOAD)) {
+    base::string16 title = web_contents_->GetTitle();
+    if (title_ != title) {
+      title_ = title;
+      for (auto& observer : observers_)
+        observer.OnTitleUpdated(title);
+    }
+  }
 }
 
 content::JavaScriptDialogManager* TabImpl::GetJavaScriptDialogManager(

weblayer/browser/tab_impl.h

@@ -11,6 +11,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
+#include "base/strings/string16.h"
 #include "base/timer/timer.h"
 #include "build/build_config.h"
 #include "components/find_in_page/find_result_observer.h"
@@ -251,6 +252,8 @@ class TabImpl : public Tab,
 
   const std::string guid_;
 
+  base::string16 title_;
+
   base::WeakPtrFactory<TabImpl> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(TabImpl);

weblayer/public/java/org/chromium/weblayer/Tab.java

@@ -368,6 +368,15 @@ public class Tab {
                 callback.onTabModalStateChanged(isTabModalShowing);
             }
         }
+
+        @Override
+        public void onTitleUpdated(IObjectWrapper title) {
+            StrictModeWorkaround.apply();
+            String titleString = ObjectWrapper.unwrap(title, String.class);
+            for (TabCallback callback : mCallbacks) {
+                callback.onTitleUpdated(titleString);
+            }
+        }
     }
 
     private static final class DownloadCallbackClientImpl extends IDownloadCallbackClient.Stub {

weblayer/public/java/org/chromium/weblayer/TabCallback.java

@@ -40,4 +40,12 @@ public abstract class TabCallback {
      * @since 82
      */
     public void onTabModalStateChanged(boolean isTabModalShowing) {}
+
+    /**
+     * Called when the title of this tab changes. Note before the page sets a title, the title may
+     * be a portion of the Uri.
+     * @param title New title of this tab.
+     * @since 83
+     */
+    public void onTitleUpdated(@NonNull String title) {}
 }

weblayer/public/tab_observer.h

@@ -5,6 +5,8 @@
 #ifndef WEBLAYER_PUBLIC_TAB_OBSERVER_H_
 #define WEBLAYER_PUBLIC_TAB_OBSERVER_H_
 
+#include "base/strings/string16.h"
+
 class GURL;
 
 namespace weblayer {
@@ -18,6 +20,10 @@ class TabObserver {
   // the system to reclaim memory.
   virtual void OnRenderProcessGone() {}
 
+  // Called when the title of this tab changes. Note before the page sets a
+  // title, the title may be a portion of the Uri.
+  virtual void OnTitleUpdated(const base::string16& title) {}
+
  protected:
   virtual ~TabObserver() {}
 };