[WebLayer] Update URL bar if navigation state changes.

If the navigation state of the active state changes such in a way that
indicates the URL has changed, we should update the URL bar.

Also adds a browser test to verify the expected behavior.

Bug: 1025607

Change-Id: I58431ac15fbb04d516f4a2637d4717431346fb25
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2094134
Commit-Queue: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748868}

content/shell/browser/shell.cc

@@ -514,8 +514,10 @@ bool Shell::CanOverscrollContent() {
 #endif
 }
 
-void Shell::DidNavigateMainFramePostCommit(WebContents* web_contents) {
-  PlatformSetAddressBarURL(web_contents->GetVisibleURL());
+void Shell::NavigationStateChanged(WebContents* source,
+                                   InvalidateTypes changed_flags) {
+  if (changed_flags & INVALIDATE_TYPE_URL)
+    PlatformSetAddressBarURL(source->GetVisibleURL());
 }
 
 JavaScriptDialogManager* Shell::GetJavaScriptDialogManager(

content/shell/browser/shell.h

@@ -161,7 +161,8 @@ class Shell : public WebContentsDelegate,
                           bool last_unlocked_by_target) override;
   void CloseContents(WebContents* source) override;
   bool CanOverscrollContent() override;
-  void DidNavigateMainFramePostCommit(WebContents* web_contents) override;
+  void NavigationStateChanged(WebContents* source,
+                              InvalidateTypes changed_flags) override;
   JavaScriptDialogManager* GetJavaScriptDialogManager(
       WebContents* source) override;
   std::unique_ptr<BluetoothChooser> RunBluetoothChooser(

weblayer/browser/browser_impl.cc

@@ -6,6 +6,7 @@
 
 #include <algorithm>
 
+#include "base/callback_forward.h"
 #include "base/containers/unique_ptr_adapters.h"
 #include "base/memory/ptr_util.h"
 #include "base/path_service.h"
@@ -303,6 +304,9 @@ void BrowserImpl::RestoreStateIfNecessary(
 }
 
 void BrowserImpl::VisibleSecurityStateOfActiveTabChanged() {
+  if (visible_security_state_changed_callback_for_tests_)
+    std::move(visible_security_state_changed_callback_for_tests_).Run();
+
 #if defined(OS_ANDROID)
   JNIEnv* env = base::android::AttachCurrentThread();
   Java_BrowserImpl_onVisibleSecurityStateOfActiveTabChanged(env, java_impl_);

weblayer/browser/browser_impl.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/observer_list.h"
 #include "build/build_config.h"
 #include "weblayer/public/browser.h"
@@ -88,6 +89,12 @@ class BrowserImpl : public Browser {
   // Used in tests to specify a non-default max (0 means use the default).
   std::vector<uint8_t> GetMinimalPersistenceState(int max_size_in_bytes);
 
+  // Used by tests to specify a callback to listen to changes to visible
+  // security state.
+  void SetVisibleSecurityStateChangedCallback(base::OnceClosure closure) {
+    visible_security_state_changed_callback_for_tests_ = std::move(closure);
+  }
+
   // Browser:
   Tab* AddTab(std::unique_ptr<Tab> tab) override;
   std::unique_ptr<Tab> RemoveTab(Tab* tab) override;
@@ -104,6 +111,7 @@ class BrowserImpl : public Browser {
  private:
   // For creation.
   friend class Browser;
+
 #if defined(OS_ANDROID)
   friend BrowserImpl* CreateBrowserForAndroid(
       ProfileImpl*,
@@ -126,6 +134,7 @@ class BrowserImpl : public Browser {
   TabImpl* active_tab_ = nullptr;
   std::string persistence_id_;
   std::unique_ptr<BrowserPersister> browser_persister_;
+  base::OnceClosure visible_security_state_changed_callback_for_tests_;
 };
 
 }  // namespace weblayer

weblayer/browser/tab_impl.cc

@@ -444,10 +444,13 @@ content::WebContents* TabImpl::OpenURLFromTab(
   return source;
 }
 
-void TabImpl::DidNavigateMainFramePostCommit(
-    content::WebContents* web_contents) {
-  for (auto& observer : observers_)
-    observer.DisplayedUrlChanged(web_contents->GetVisibleURL());
+void TabImpl::NavigationStateChanged(content::WebContents* source,
+                                     content::InvalidateTypes changed_flags) {
+  if (changed_flags & content::INVALIDATE_TYPE_URL) {
+    for (auto& observer : observers_)
+      observer.DisplayedUrlChanged(source->GetVisibleURL());
+    UpdateBrowserVisibleSecurityStateIfNecessary();
+  }
 }
 
 content::JavaScriptDialogManager* TabImpl::GetJavaScriptDialogManager(
@@ -640,6 +643,10 @@ void TabImpl::OnFindResultAvailable(content::WebContents* web_contents) {
 }
 
 void TabImpl::DidChangeVisibleSecurityState() {
+  UpdateBrowserVisibleSecurityStateIfNecessary();
+}
+
+void TabImpl::UpdateBrowserVisibleSecurityStateIfNecessary() {
   if (browser_) {
     if (browser_->GetActiveTab() == this)
       browser_->VisibleSecurityStateOfActiveTabChanged();

weblayer/browser/tab_impl.h

@@ -153,8 +153,8 @@ class TabImpl : public Tab,
   content::WebContents* OpenURLFromTab(
       content::WebContents* source,
       const content::OpenURLParams& params) override;
-  void DidNavigateMainFramePostCommit(
-      content::WebContents* web_contents) override;
+  void NavigationStateChanged(content::WebContents* source,
+                              content::InvalidateTypes changed_flags) override;
   content::JavaScriptDialogManager* GetJavaScriptDialogManager(
       content::WebContents* web_contents) override;
   content::ColorChooser* OpenColorChooser(
@@ -225,6 +225,8 @@ class TabImpl : public Tab,
                                     content::BrowserControlsState constraint);
 #endif
 
+  void UpdateBrowserVisibleSecurityStateIfNecessary();
+
   BrowserImpl* browser_ = nullptr;
   DownloadDelegate* download_delegate_ = nullptr;
   ErrorPageDelegate* error_page_delegate_ = nullptr;

weblayer/browser/urlbar/url_bar_browsertest.cc

+// Copyright 2020 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 "base/callback_forward.h"
+#include "base/run_loop.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "weblayer/browser/browser_impl.h"
+#include "weblayer/browser/profile_impl.h"
+#include "weblayer/browser/tab_impl.h"
+#include "weblayer/test/weblayer_browser_test.h"
+#include "weblayer/test/weblayer_browser_test_utils.h"
+
+namespace weblayer {
+
+class UrlBarBrowserTest : public WebLayerBrowserTest {
+ public:
+  UrlBarBrowserTest() = default;
+  ~UrlBarBrowserTest() override = default;
+
+  // WebLayerBrowserTest
+  void SetUpOnMainThread() override {
+    WebLayerBrowserTest::SetUpOnMainThread();
+    ASSERT_TRUE(embedded_test_server()->Start());
+    browser_ = Browser::Create(GetProfile(), nullptr);
+    tab_ = static_cast<TabImpl*>(browser_->AddTab(Tab::Create(GetProfile())));
+    browser_->SetActiveTab(tab_);
+  }
+  void PostRunTestOnMainThread() override {
+    tab_ = nullptr;
+    browser_.reset();
+    WebLayerBrowserTest::PostRunTestOnMainThread();
+  }
+
+  GURL real_url() {
+    return embedded_test_server()->GetURL("/simple_page.html");
+  }
+  GURL abort_url() { return embedded_test_server()->GetURL("/nocontent"); }
+
+  void SetVisibleSecurityStateChangedCallback(base::OnceClosure closure) {
+    browser_impl()->SetVisibleSecurityStateChangedCallback(std::move(closure));
+  }
+
+ protected:
+  TabImpl* tab_ = nullptr;
+
+ private:
+  std::unique_ptr<Browser> browser_;
+  BrowserImpl* browser_impl() {
+    return static_cast<BrowserImpl*>(browser_.get());
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(UrlBarBrowserTest, CanceledNavigationsUpdateUrl) {
+  NavigateAndWaitForCompletion(real_url(), tab_);
+
+  {
+    base::RunLoop run_loop;
+    SetVisibleSecurityStateChangedCallback(run_loop.QuitClosure());
+
+    // Navigating to the /nocontent url cancels the navigation with a 204 error.
+    NavigateAndWaitForStart(abort_url(), tab_);
+
+    // The test won't finish until WebLayer acts on the resulting
+    // WebContentsObserver::DidChangeVisibleSecurityState() notification, or the
+    // test times out.
+    run_loop.Run();
+  }
+}
+
+}  // namespace weblayer

weblayer/test/BUILD.gn

@@ -177,6 +177,7 @@ test("weblayer_browsertests") {
       # integration tests.
       "../browser/persistence/browser_persister_browsertest.cc",
       "../browser/persistence/minimal_browser_persister_browsertest.cc",
+      "../browser/urlbar/url_bar_browsertest.cc",
     ]
   }
 }

weblayer/test/weblayer_browser_test_utils.cc

@@ -44,6 +44,11 @@ void NavigateAndWaitForFailure(const GURL& url, Shell* shell) {
                           TestNavigationObserver::NavigationEvent::kFailure);
 }
 
+void NavigateAndWaitForStart(const GURL& url, Tab* tab) {
+  NavigateAndWaitForEvent(url, tab,
+                          TestNavigationObserver::NavigationEvent::kStart);
+}
+
 base::Value ExecuteScript(Shell* shell,
                           const std::string& script,
                           bool use_separate_isolate) {

weblayer/test/weblayer_browser_test_utils.h

@@ -28,6 +28,9 @@ void NavigateAndWaitForCompletion(const GURL& url, Tab* tab);
 // Navigates |shell| to |url| and wait for failed navigation.
 void NavigateAndWaitForFailure(const GURL& url, Shell* shell);
 
+// Initiates navigation to |url| in |tab| and waits for it to start.
+void NavigateAndWaitForStart(const GURL& url, Tab* tab);
+
 // Executes |script| in |shell| and returns the result.
 base::Value ExecuteScript(Shell* shell,
                           const std::string& script,