Auto merge of #24634 - paulrouget:url, r=jdm

Do not assume UWP and Servo agree on URL validity

This should address #24589.

Note: to check that a URL is valid, we try to parse the url and if an exception is raised, we assume the url is not valid. Even though we catch the exception, Visual Studio might "force" the crash + break here: https://github.com/servo/servo/compare/master...paulrouget:url?expand=1#diff-4e059561062fe024a35522f60f7f14c1R116 - I'm not sure why. In VS, in the Exception Setting, we need to uncheck the Break When Thrown option to avoid that crash. It only happens for few urls and I'm unclear why.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #24589 (GitHub issue number if applicable)

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

7 files changed, 67 insertions, 35 deletions

diff --git a/ports/libsimpleservo/api/src/lib.rs b/ports/libsimpleservo/api/src/lib.rs
index 4f17dad9e7a..2850911f230 100644
--- a/ports/libsimpleservo/api/src/lib.rs
+++ b/ports/libsimpleservo/api/src/lib.rs
@@ -152,6 +152,12 @@ pub fn servo_version() -> String {
     servo::config::servo_version()
 }
 
+/// Test if a url is valid.
+pub fn is_uri_valid(url: &str) -> bool {
+    info!("load_uri: {}", url);
+    ServoUrl::parse(url).is_ok()
+}
+
 /// Initialize Servo. At that point, we need a valid GL context.
 /// In the future, this will be done in multiple steps.
 pub fn init(
diff --git a/ports/libsimpleservo/capi/src/lib.rs b/ports/libsimpleservo/capi/src/lib.rs
index 32f3b368ea6..c6b27ae6d48 100644
--- a/ports/libsimpleservo/capi/src/lib.rs
+++ b/ports/libsimpleservo/capi/src/lib.rs
@@ -186,16 +186,17 @@ fn do_redirect_stdout_stderr() -> Result<(), ()> {
     Ok(())
 }
 
-fn call<F>(f: F)
+fn call<T, F>(f: F) -> T
 where
-    F: Fn(&mut ServoGlue) -> Result<(), &'static str>,
+    F: Fn(&mut ServoGlue) -> Result<T, &'static str>,
 {
-    if let Err(e) = SERVO.with(|s| match s.borrow_mut().as_mut() {
+    match SERVO.with(|s| match s.borrow_mut().as_mut() {
         Some(ref mut s) => (f)(s),
         None => Err("Servo not available in this thread"),
     }) {
-        panic!(e);
-    };
+        Err(e) => panic!(e),
+        Ok(r) => r,
+    }
 }
 
 /// Callback used by Servo internals
@@ -430,13 +431,23 @@ pub extern "C" fn perform_updates() {
 }
 
 #[no_mangle]
-pub extern "C" fn load_uri(url: *const c_char) {
+pub extern "C" fn is_uri_valid(url: *const c_char) -> bool {
+    catch_any_panic(|| {
+        debug!("is_uri_valid");
+        let url = unsafe { CStr::from_ptr(url) };
+        let url = url.to_str().expect("Can't read string");
+        simpleservo::is_uri_valid(url)
+    })
+}
+
+#[no_mangle]
+pub extern "C" fn load_uri(url: *const c_char) -> bool {
     catch_any_panic(|| {
         debug!("load_url");
         let url = unsafe { CStr::from_ptr(url) };
         let url = url.to_str().expect("Can't read string");
-        call(|s| s.load_uri(url));
-    });
+        call(|s| Ok(s.load_uri(url).is_ok()))
+    })
 }
 
 #[no_mangle]
diff --git a/support/hololens/ServoApp/BrowserPage.cpp b/support/hololens/ServoApp/BrowserPage.cpp
index 98521ccd7da..95265852b92 100644
--- a/support/hololens/ServoApp/BrowserPage.cpp
+++ b/support/hololens/ServoApp/BrowserPage.cpp
@@ -116,7 +116,7 @@ void BrowserPage::OnURLEdited(IInspectable const &,
     servoControl().Focus(FocusState::Programmatic);
     auto input = urlTextbox().Text();
     auto uri = servoControl().LoadURIOrSearch(input);
-    urlTextbox().Text(uri.ToString());
+    urlTextbox().Text(uri);
   }
 }
 
diff --git a/support/hololens/ServoApp/ServoControl/Servo.h b/support/hololens/ServoApp/ServoControl/Servo.h
index b89ca1c8d76..a27313d3792 100644
--- a/support/hololens/ServoApp/ServoControl/Servo.h
+++ b/support/hololens/ServoApp/ServoControl/Servo.h
@@ -66,7 +66,10 @@ public:
 
   void Reload() { capi::reload(); }
   void Stop() { capi::stop(); }
-  void LoadUri(hstring uri) { capi::load_uri(*hstring2char(uri)); }
+  bool LoadUri(hstring uri) { return capi::load_uri(*hstring2char(uri)); }
+  bool IsUriValid(hstring uri) {
+    return capi::is_uri_valid(*hstring2char(uri));
+  }
   void Scroll(float dx, float dy, float x, float y) {
     capi::scroll((int32_t)dx, (int32_t)dy, (int32_t)x, (int32_t)y);
   }
diff --git a/support/hololens/ServoApp/ServoControl/ServoControl.cpp b/support/hololens/ServoApp/ServoControl/ServoControl.cpp
index c8444b57d2d..94ea29bfad7 100644
--- a/support/hololens/ServoApp/ServoControl/ServoControl.cpp
+++ b/support/hololens/ServoApp/ServoControl/ServoControl.cpp
@@ -195,25 +195,43 @@ void ServoControl::Reload() {
 void ServoControl::Stop() {
   RunOnGLThread([=] { mServo->Stop(); });
 }
-Uri ServoControl::LoadURIOrSearch(hstring input) {
-  auto uri = TryParseURI(input);
-  if (uri == std::nullopt) {
-    bool has_dot = wcsstr(input.c_str(), L".") != nullptr;
-    hstring input2 = L"https://" + input;
-    uri = TryParseURI(input2);
-    if (uri == std::nullopt || !has_dot) {
-      hstring input3 =
-          L"https://duckduckgo.com/html/?q=" + Uri::EscapeComponent(input);
-      uri = TryParseURI(input3);
-    }
+hstring ServoControl::LoadURIOrSearch(hstring input) {
+  // Initial input is valid
+  if (mServo->IsUriValid(input)) {
+    TryLoadUri(input);
+    return input;
+  }
+
+  // Not valid. Maybe it's just missing the scheme.
+  hstring with_scheme = L"https://" + input;
+  // If the user only types "mozilla" we don't want to go to
+  // https://mozilla even though it's a valid url.
+  bool has_dot = wcsstr(input.c_str(), L".") != nullptr;
+  if (mServo->IsUriValid(with_scheme) && has_dot) {
+    TryLoadUri(with_scheme);
+    return with_scheme;
   }
-  auto finalUri = uri.value();
+
+  // Doesn't look like a URI. Let's search for the string.
+  hstring searchUri =
+      L"https://duckduckgo.com/html/?q=" + Uri::EscapeComponent(input);
+  TryLoadUri(searchUri);
+  return searchUri;
+}
+
+void ServoControl::TryLoadUri(hstring input) {
   if (!mLooping) {
-    mInitialURL = finalUri.ToString();
+    mInitialURL = input;
   } else {
-    RunOnGLThread([=] { mServo->LoadUri(finalUri.ToString()); });
+    RunOnGLThread([=] {
+      if (!mServo->LoadUri(input)) {
+        RunOnUIThread([=] {
+          Windows::UI::Popups::MessageDialog msg{L"URI not valid"};
+          msg.ShowAsync();
+        });
+      }
+    });
   }
-  return finalUri;
 }
 
 void ServoControl::RunOnGLThread(std::function<void()> task) {
diff --git a/support/hololens/ServoApp/ServoControl/ServoControl.h b/support/hololens/ServoApp/ServoControl/ServoControl.h
index 77645f6ffaf..921cf767021 100644
--- a/support/hololens/ServoApp/ServoControl/ServoControl.h
+++ b/support/hololens/ServoApp/ServoControl/ServoControl.h
@@ -14,7 +14,7 @@ struct ServoControl : ServoControlT<ServoControl>, public servo::ServoDelegate {
   void Reload();
   void Stop();
   void Shutdown();
-  Windows::Foundation::Uri LoadURIOrSearch(hstring);
+  hstring LoadURIOrSearch(hstring);
 
   void OnLoaded(IInspectable const &,
                 Windows::UI::Xaml::RoutedEventArgs const &);
@@ -110,14 +110,6 @@ private:
   void StopRenderLoop();
   void Loop();
 
-  std::optional<Windows::Foundation::Uri> TryParseURI(hstring input) {
-    try {
-      return Windows::Foundation::Uri(input);
-    } catch (hresult_invalid_argument const &) {
-      return {};
-    }
-  }
-
   void OnSurfaceTapped(IInspectable const &,
                        Windows::UI::Xaml::Input::TappedRoutedEventArgs const &);
 
@@ -147,6 +139,8 @@ private:
   template <typename Callable> void RunOnUIThread(Callable);
   void RunOnGLThread(std::function<void()>);
 
+  void TryLoadUri(hstring);
+
   std::unique_ptr<servo::Servo> mServo;
   EGLSurface mRenderSurface{EGL_NO_SURFACE};
   OpenGLES mOpenGLES;
diff --git a/support/hololens/ServoApp/ServoControl/ServoControl.idl b/support/hololens/ServoApp/ServoControl/ServoControl.idl
index 597fe3320da..f6ba55b3b56 100644
--- a/support/hololens/ServoApp/ServoControl/ServoControl.idl
+++ b/support/hololens/ServoApp/ServoControl/ServoControl.idl
@@ -9,7 +9,7 @@ namespace ServoApp {
       void GoForward();
       void Reload();
       void Stop();
-      Windows.Foundation.Uri LoadURIOrSearch(String url);
+      String LoadURIOrSearch(String url);
       void SetTransientMode(Boolean transient);
       void SetArgs(String args);
       void Shutdown();