[iOS] Replace WebState::GetCurrentURL with GetLastCommittedURLIfTrusted

WebState::GetCurrentURL is a confusing API that doesn't match any concepts in WebContents. Callers typically instead need the last committed URL. Since WebKit doesn't directly expose a last committed URL, a notion of trust is still needed, since there are cases where WebState can't be certain about the true last committed URL. This CL introduces a GetLastCommittedURLIfTrusted method on WebState and migrates most callers of GetCurrentURL to this new method. This method returns an absl::optional<GURL> which is absl::nullopt when the last committed URL is not trusted. NavigationManagerImpl::GetLastCommittedItemInCurrentOrRestoredSession continues to need GetCurrentURL but doesn't use the `trust_level` argument passed to it, so this CL moves GetCurrentURL to NavigationManagerDelegate where it can only be used by NavigationManagerImpl. With these changes, the URLVerificationTrustLevel concept becomes unused, so this CL deletes this, leading to more simplification in CRWWebController. Bug: 457679 Change-Id: If4d584e47490232ae72025005146e0268ddb6d15 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4571221 Reviewed-by: Justin Cohen <justincohen@chromium.org> Reviewed-by: Gauthier Ambard <gambard@chromium.org> Commit-Queue: Ali Juma <ajuma@chromium.org> Reviewed-by: Olivier Robin <olivierrobin@chromium.org> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Cr-Commit-Position: refs/heads/main@{#1151988}
chromium · Jun 1, 2023 · c56374e · c56374e
1 parent e90c049
commit c56374e
Show file tree

Hide file tree

Showing 27 changed files with 68 additions and 167 deletions.
diff --git a/components/autofill/ios/browser/autofill_agent.mm b/components/autofill/ios/browser/autofill_agent.mm
@@ -55,7 +55,6 @@
 #import "components/prefs/pref_service.h"
 #import "components/ukm/ios/ukm_url_recorder.h"
 #import "ios/web/common/url_scheme_util.h"
-#import "ios/web/public/deprecated/url_verification_constants.h"
 #import "ios/web/public/js_messaging/web_frame.h"
 #import "ios/web/public/js_messaging/web_frames_manager.h"
 #import "ios/web/public/js_messaging/web_frames_manager_observer_bridge.h"

diff --git a/components/password_manager/ios/password_form_helper.mm b/components/password_manager/ios/password_form_helper.mm
@@ -46,13 +46,15 @@
 namespace password_manager {
 bool GetPageURLAndCheckTrustLevel(web::WebState* web_state,
                                   GURL* __nullable page_url) {
-  auto trustLevel = web::URLVerificationTrustLevel::kNone;
-  GURL dummy;
-  if (!page_url) {
-    page_url = &dummy;
+  absl::optional<GURL> last_committed_url =
+      web_state->GetLastCommittedURLIfTrusted();
+  if (last_committed_url) {
+    if (page_url) {
+      *page_url = std::move(*last_committed_url);
+    }
+    return true;
   }
-  *page_url = web_state->GetCurrentURL(&trustLevel);
-  return trustLevel == web::URLVerificationTrustLevel::kAbsolute;
+  return false;
 }
 
 // The frame id associated with the frame which sent to form message.

diff --git a/ios/chrome/browser/passwords/password_controller_js_unittest.mm b/ios/chrome/browser/passwords/password_controller_js_unittest.mm
@@ -9,7 +9,6 @@
 #import "components/password_manager/ios/password_manager_java_script_feature.h"
 #import "ios/chrome/browser/shared/model/browser_state/test_chrome_browser_state.h"
 #import "ios/chrome/browser/web/chrome_web_client.h"
-#import "ios/web/public/deprecated/url_verification_constants.h"
 #import "ios/web/public/js_messaging/web_frame.h"
 #import "ios/web/public/js_messaging/web_frames_manager.h"
 #import "ios/web/public/test/js_test_util.h"
@@ -102,10 +101,7 @@ bool SetUpUniqueIDs() {
     return result;
   }
 
-  std::string BaseUrl() {
-    web::URLVerificationTrustLevel unused_level;
-    return web_state()->GetCurrentURL(&unused_level).spec();
-  }
+  std::string BaseUrl() { return web_state()->GetLastCommittedURL().spec(); }
 
  protected:
   id ExecuteJavaScript(NSString* script) {

diff --git a/ios/chrome/browser/passwords/password_controller_unittest.mm b/ios/chrome/browser/passwords/password_controller_unittest.mm
@@ -47,7 +47,6 @@
 #import "ios/chrome/browser/shared/model/browser_state/test_chrome_browser_state.h"
 #import "ios/chrome/browser/ui/autofill/form_input_accessory/form_input_accessory_mediator.h"
 #import "ios/chrome/browser/web/chrome_web_client.h"
-#import "ios/web/public/deprecated/url_verification_constants.h"
 #import "ios/web/public/js_messaging/web_frame.h"
 #import "ios/web/public/js_messaging/web_frames_manager.h"
 #import "ios/web/public/navigation/navigation_item.h"
@@ -472,8 +471,7 @@ void LoadHtml(NSString* html, const GURL& url) {
   }
 
   std::string BaseUrl() const {
-    web::URLVerificationTrustLevel unused_level;
-    return web_state()->GetCurrentURL(&unused_level).spec();
+    return web_state()->GetLastCommittedURL().spec();
   }
 
   web::WebState* web_state() const { return web_state_.get(); }

diff --git a/ios/chrome/browser/passwords/well_known_change_password_tab_helper_unittest.mm b/ios/chrome/browser/passwords/well_known_change_password_tab_helper_unittest.mm
@@ -172,9 +172,7 @@ void SetChangePasswordURLForAffiliationService(
 };
 
 GURL WellKnownChangePasswordTabHelperTest::GetNavigatedUrl() const {
-  web::URLVerificationTrustLevel trust_level =
-      web::URLVerificationTrustLevel::kAbsolute;
-  GURL url = web_state()->GetCurrentURL(&trust_level);
+  GURL url = web_state()->GetLastCommittedURL();
   // When redirecting with WebState::OpenURL() `web_state_` is not
   // updated, we only see the registered request in
   // FakeWebStateDelegate::last_open_url_request().

diff --git a/ios/chrome/browser/ui/autofill/form_input_accessory/form_input_accessory_mediator.mm b/ios/chrome/browser/ui/autofill/form_input_accessory/form_input_accessory_mediator.mm
@@ -273,15 +273,14 @@ - (void)webState:(web::WebState*)webState
   }
 
   // Return early if the URL can't be verified.
-  web::URLVerificationTrustLevel trustLevel;
-  const GURL pageURL(webState->GetCurrentURL(&trustLevel));
-  if (trustLevel != web::URLVerificationTrustLevel::kAbsolute) {
+  absl::optional<GURL> pageURL = webState->GetLastCommittedURLIfTrusted();
+  if (!pageURL) {
     [self reset];
     return;
   }
 
   // Return early, pause and reset if the url is not HTML.
-  if (!web::UrlHasWebScheme(pageURL) || !webState->ContentIsHTML()) {
+  if (!web::UrlHasWebScheme(*pageURL) || !webState->ContentIsHTML()) {
     [self reset];
     return;
   }
@@ -415,14 +414,13 @@ - (BOOL)isInputAccessoryViewActive {
   }
 
   // Return early if the URL can't be verified.
-  web::URLVerificationTrustLevel trustLevel;
-  const GURL pageURL(_webState->GetCurrentURL(&trustLevel));
-  if (trustLevel != web::URLVerificationTrustLevel::kAbsolute) {
+  absl::optional<GURL> pageURL = _webState->GetLastCommittedURLIfTrusted();
+  if (!pageURL) {
     return NO;
   }
 
   // Return early if the url is not HTML.
-  if (!web::UrlHasWebScheme(pageURL) || !_webState->ContentIsHTML()) {
+  if (!web::UrlHasWebScheme(*pageURL) || !_webState->ContentIsHTML()) {
     return NO;
   }
 

diff --git a/ios/web/content/web_state/content_web_state.h b/ios/web/content/web_state/content_web_state.h
@@ -104,7 +104,7 @@ class ContentWebState : public WebState,
   int GetNavigationItemCount() const override;
   const GURL& GetVisibleURL() const override;
   const GURL& GetLastCommittedURL() const override;
-  GURL GetCurrentURL(URLVerificationTrustLevel* trust_level) const override;
+  absl::optional<GURL> GetLastCommittedURLIfTrusted() const override;
   WebFramesManager* GetWebFramesManager(ContentWorld world) override;
   CRWWebViewProxyType GetWebViewProxy() const override;
   void AddObserver(WebStateObserver* observer) override;

diff --git a/ios/web/content/web_state/content_web_state.mm b/ios/web/content/web_state/content_web_state.mm
@@ -347,12 +347,8 @@ void RegisterAllowedCertificate(
   return item ? item->GetURL() : GURL::EmptyGURL();
 }
 
-GURL ContentWebState::GetCurrentURL(
-    URLVerificationTrustLevel* trust_level) const {
-  // TODO(crbug.com/1419001): Make sure that callers are using this correctly
-  // and that unexpected URLs are not displayed.
-  auto* item = navigation_manager_->GetLastCommittedItem();
-  return item ? item->GetURL() : GURL::EmptyGURL();
+absl::optional<GURL> ContentWebState::GetLastCommittedURLIfTrusted() const {
+  return GetLastCommittedURL();
 }
 
 WebFramesManager* ContentWebState::GetWebFramesManager(ContentWorld world) {

diff --git a/ios/web/navigation/navigation_manager_delegate.h b/ios/web/navigation/navigation_manager_delegate.h
@@ -8,6 +8,7 @@
 #include <stddef.h>
 
 #include "ios/web/common/user_agent.h"
+#import "url/gurl.h"
 
 @protocol CRWWebViewNavigationProxy;
 @class WKBackForwardListItem;
@@ -71,6 +72,11 @@ class NavigationManagerDelegate {
 
   // Used to access pending item stored in NavigationContext.
   virtual NavigationItemImpl* GetPendingItem() = 0;
+
+  // Returns the NavigationManagerDelegate's view of the current URL. This is
+  // used as a fallback in situations where the NavigationManager doesn't trust
+  // its own view of the last committed item.
+  virtual GURL GetCurrentURL() const = 0;
 };
 
 }  // namespace web

diff --git a/ios/web/navigation/navigation_manager_impl.mm b/ios/web/navigation/navigation_manager_impl.mm
@@ -1019,7 +1019,7 @@ void RecordSessionRestorationResultForSource(
     // Don't check trust level here, as at this point it's expected
     // the _documentURL and the last_commited_item URL have an origin
     // mismatch.
-    GURL document_url = GetWebState()->GetCurrentURL(/*trust_level=*/nullptr);
+    GURL document_url = delegate_->GetCurrentURL();
     if (!last_committed_web_view_item_) {
       last_committed_web_view_item_ = CreateNavigationItemWithRewriters(
           /*url=*/GURL::EmptyGURL(), Referrer(),

diff --git a/ios/web/navigation/navigation_manager_impl_unittest.mm b/ios/web/navigation/navigation_manager_impl_unittest.mm
@@ -97,6 +97,7 @@ void RemoveWebView() override {
                     NavigationInitiationType,
                     bool));
   MOCK_METHOD0(GetPendingItem, NavigationItemImpl*());
+  MOCK_CONST_METHOD0(GetCurrentURL, GURL());
 
  private:
   WebState* GetWebState() override { return web_state_; }

diff --git a/ios/web/public/deprecated/BUILD.gn b/ios/web/public/deprecated/BUILD.gn
@@ -12,8 +12,5 @@ source_set("deprecated") {
     "//ios/web/public:web_state_observer",
     "//url",
   ]
-  sources = [
-    "global_web_state_observer.h",
-    "url_verification_constants.h",
-  ]
+  sources = [ "global_web_state_observer.h" ]
 }
diff --git a/ios/web/public/deprecated/url_verification_constants.h b/ios/web/public/deprecated/url_verification_constants.h
diff --git a/ios/web/public/test/fakes/fake_web_state.h b/ios/web/public/test/fakes/fake_web_state.h
@@ -14,7 +14,6 @@
 
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
-#include "ios/web/public/deprecated/url_verification_constants.h"
 #import "ios/web/public/favicon/favicon_status.h"
 #import "ios/web/public/navigation/navigation_manager.h"
 #import "ios/web/public/navigation/web_state_policy_decider.h"
@@ -86,7 +85,7 @@ class FakeWebState : public WebState {
   int GetNavigationItemCount() const override;
   const GURL& GetVisibleURL() const override;
   const GURL& GetLastCommittedURL() const override;
-  GURL GetCurrentURL(URLVerificationTrustLevel* trust_level) const override;
+  absl::optional<GURL> GetLastCommittedURLIfTrusted() const override;
   CRWWebViewProxyType GetWebViewProxy() const override;
 
   void AddObserver(WebStateObserver* observer) override;
@@ -136,7 +135,6 @@ class FakeWebState : public WebState {
   void SetCurrentURL(const GURL& url);
   void SetNavigationItemCount(int count);
   void SetVisibleURL(const GURL& url);
-  void SetTrustLevel(URLVerificationTrustLevel trust_level);
   void SetNavigationManager(
       std::unique_ptr<NavigationManager> navigation_manager);
   void SetWebFramesManager(
@@ -199,7 +197,6 @@ class FakeWebState : public WebState {
   FaviconStatus favicon_status_;
   GURL url_;
   std::u16string title_;
-  URLVerificationTrustLevel trust_level_ = kAbsolute;
   bool content_is_html_ = true;
   std::string mime_type_;
   std::unique_ptr<NavigationManager> navigation_manager_;

diff --git a/ios/web/public/test/fakes/fake_web_state.mm b/ios/web/public/test/fakes/fake_web_state.mm
@@ -259,10 +259,7 @@
   return url_;
 }
 
-GURL FakeWebState::GetCurrentURL(URLVerificationTrustLevel* trust_level) const {
-  if (trust_level) {
-    *trust_level = trust_level_;
-  }
+absl::optional<GURL> FakeWebState::GetLastCommittedURLIfTrusted() const {
   return url_;
 }
 
@@ -452,10 +449,6 @@
   url_ = url;
 }
 
-void FakeWebState::SetTrustLevel(URLVerificationTrustLevel trust_level) {
-  trust_level_ = trust_level;
-}
-
 void FakeWebState::SetCanTakeSnapshot(bool can_take_snapshot) {
   can_take_snapshot_ = can_take_snapshot;
 }

diff --git a/ios/web/public/test/web_test_with_web_state.mm b/ios/web/public/test/web_test_with_web_state.mm
@@ -12,7 +12,6 @@
 #import "ios/web/common/features.h"
 #import "ios/web/navigation/navigation_manager_impl.h"
 #import "ios/web/navigation/wk_navigation_util.h"
-#import "ios/web/public/deprecated/url_verification_constants.h"
 #import "ios/web/public/test/js_test_util.h"
 #import "ios/web/public/test/task_observer_util.h"
 #import "ios/web/public/test/web_state_test_util.h"
@@ -143,8 +142,7 @@
 }
 
 std::string WebTestWithWebState::BaseUrl() const {
-  web::URLVerificationTrustLevel unused_level;
-  return web_state()->GetCurrentURL(&unused_level).spec();
+  return web_state()->GetLastCommittedURL().spec();
 }
 
 web::WebState* WebTestWithWebState::web_state() {

diff --git a/ios/web/public/web_state.h b/ios/web/public/web_state.h
@@ -22,7 +22,6 @@
 #include "base/time/time.h"
 #include "build/blink_buildflags.h"
 #include "components/sessions/core/session_id.h"
-#include "ios/web/public/deprecated/url_verification_constants.h"
 #include "ios/web/public/js_messaging/content_world.h"
 #include "ios/web/public/navigation/referrer.h"
 #include "mojo/public/cpp/bindings/generic_pending_receiver.h"
@@ -393,12 +392,9 @@ class WebState : public base::SupportsUserData {
   // displayed in this WebState. It represents the current security context.
   virtual const GURL& GetLastCommittedURL() const = 0;
 
-  // Returns the WebState view of the current URL. Moreover, this method
-  // will set the trustLevel enum to the appropriate level from a security point
-  // of view. The caller has to handle the case where `trust_level` is not
-  // appropriate.  Passing `null` will skip the trust check.
-  // TODO(crbug.com/457679): Figure out a clean API for this.
-  virtual GURL GetCurrentURL(URLVerificationTrustLevel* trust_level) const = 0;
+  // Returns the last committed URL if the correctness of this URL's origin is
+  // trusted, and absl::nullopt otherwise.
+  virtual absl::optional<GURL> GetLastCommittedURLIfTrusted() const = 0;
 
   // Returns the current CRWWebViewProxy object.
   virtual CRWWebViewProxyType GetWebViewProxy() const = 0;

diff --git a/ios/web/test/fakes/fake_navigation_manager_delegate.h b/ios/web/test/fakes/fake_navigation_manager_delegate.h
@@ -28,6 +28,7 @@ class FakeNavigationManagerDelegate : public NavigationManagerDelegate {
                                bool has_user_gesture) override;
   void RemoveWebView() override;
   NavigationItemImpl* GetPendingItem() override;
+  GURL GetCurrentURL() const override;
 
   // Setters for tests to inject dependencies.
   void SetWebViewNavigationProxy(id test_web_view);

diff --git a/ios/web/test/fakes/fake_navigation_manager_delegate.mm b/ios/web/test/fakes/fake_navigation_manager_delegate.mm
@@ -40,6 +40,10 @@
   return nullptr;
 }
 
+GURL FakeNavigationManagerDelegate::GetCurrentURL() const {
+  return GURL();
+}
+
 void FakeNavigationManagerDelegate::SetWebViewNavigationProxy(id web_view) {
   test_web_view_ = web_view;
 }

diff --git a/ios/web/web_state/ui/crw_web_controller.h b/ios/web/web_state/ui/crw_web_controller.h
@@ -8,7 +8,6 @@
 #import <UIKit/UIKit.h>
 
 #import "base/values.h"
-#include "ios/web/public/deprecated/url_verification_constants.h"
 #import "ios/web/web_state/ui/crw_touch_tracking_recognizer.h"
 #import "ios/web/web_state/ui/crw_web_view_navigation_proxy.h"
 
@@ -116,11 +115,9 @@ class WebStateImpl;
 // Returns YES if the current live view is a web view with HTML.
 - (BOOL)contentIsHTML;
 
-// Returns the CRWWebController's view of the current URL. Moreover, this method
-// will set the trustLevel enum to the appropriate level from a security point
-// of view. The caller has to handle the case where `trustLevel` is not
-// appropriate, as this method won't display any error to the user.
-- (GURL)currentURLWithTrustLevel:(web::URLVerificationTrustLevel*)trustLevel;
+// Returns the CRWWebController's view of the current URL. During navigations,
+// this may not be the same as the navigation manager's view of the current URL.
+- (GURL)currentURL;
 
 // Reloads web view. `isRendererInitiated` is YES for renderer-initiated
 // navigation. `isRendererInitiated` is NO for browser-initiated navigation.