Auto merge of #17083 - danielj41:javascript-url-global-3, r=jdm

"javascript:" urls: execute in correct global scope

<!-- Please describe your changes on the following line: -->

#### Summary

This pull request makes `javascript:` urls execute in the correct global scope.

#### Example

```html
<script> var x = 4; </script>

<!-- this branch: logs "4" -->
<!-- master: undefined variable error -->
<a href="javascript:console.log(x)">link</a>
```

#### Questions

I'm new to servo and rust, so I'm unsure about these changes. In particular:
  * What's the appropriate place to evaluate the js?
    * I moved it to `handle_navigate`, but I'm not sure if this will catch all occurrences of `javascript:` urls. I also don't know if this will execute in the correct thread and the correct window.
  * What should I do with the result of the js evaluation?
    * I just ignored it. The previous behavior displayed it as the content of a new page load.

---
<!-- 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 #15147, #16718

<!-- Either: -->
- [x] There are tests for these changes

<!-- 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. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17083)
<!-- Reviewable:end -->

components/constellation/constellation.rs

@@ -1067,6 +1067,10 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                 debug!("constellation got URL load message from script");
                 self.handle_load_url_msg(source_top_ctx_id, source_pipeline_id, load_data, replace);
             }
+            FromScriptMsg::AbortLoadUrl => {
+                debug!("constellation got abort URL load message from script");
+                self.handle_abort_load_url_msg(source_pipeline_id);
+            }
             // A page loaded has completed all parsing, script, and reflow messages have been sent.
             FromScriptMsg::LoadComplete => {
                 debug!("constellation got load complete message");
@@ -1891,6 +1895,18 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         }
     }
 
+    fn handle_abort_load_url_msg(&mut self, new_pipeline_id: PipelineId) {
+        let pending_index = self.pending_changes.iter().rposition(|change| {
+            change.new_pipeline_id == new_pipeline_id
+        });
+
+        // If it is found, remove it from the pending changes.
+        if let Some(pending_index) = pending_index {
+            self.pending_changes.remove(pending_index);
+            self.close_pipeline(new_pipeline_id, DiscardBrowsingContext::No, ExitPipelineMode::Normal);
+        }
+    }
+
     fn handle_load_start_msg(&mut self, top_level_browsing_context_id: TopLevelBrowsingContextId,
                              pipeline_id: PipelineId) {
         if self.pipelines.get(&pipeline_id).and_then(|p| p.parent_info).is_none() {

components/script/dom/htmliframeelement.rs

@@ -44,7 +44,7 @@ use msg::constellation_msg::{FrameType, BrowsingContextId, PipelineId, TopLevelB
 use net_traits::response::HttpsState;
 use script_layout_interface::message::ReflowQueryType;
 use script_thread::{ScriptThread, Runnable};
-use script_traits::{IFrameLoadInfo, IFrameLoadInfoWithData, LoadData, UpdatePipelineIdReason};
+use script_traits::{IFrameLoadInfo, IFrameLoadInfoWithData, JsEvalResult, LoadData, UpdatePipelineIdReason};
 use script_traits::{MozBrowserEvent, NewLayoutInfo, ScriptMsg};
 use script_traits::IFrameSandboxState::{IFrameSandboxed, IFrameUnsandboxed};
 use servo_atoms::Atom;
@@ -114,7 +114,7 @@ impl HTMLIFrameElement {
     }
 
     pub fn navigate_or_reload_child_browsing_context(&self,
-                                                     load_data: Option<LoadData>,
+                                                     mut load_data: Option<LoadData>,
                                                      nav_type: NavigationType,
                                                      replace: bool) {
         let sandboxed = if self.is_sandboxed() {
@@ -140,11 +140,26 @@ impl HTMLIFrameElement {
         // document; the new navigation will continue blocking it.
         LoadBlocker::terminate(&mut load_blocker);
 
+        if let Some(ref mut load_data) = load_data {
+            let is_javascript = load_data.url.scheme() == "javascript";
+            if is_javascript {
+                let window_proxy = self.GetContentWindow();
+                if let Some(window_proxy) = window_proxy {
+                    ScriptThread::eval_js_url(&window_proxy.global(), load_data);
+                }
+            }
+        }
+
         //TODO(#9592): Deal with the case where an iframe is being reloaded so url is None.
         //      The iframe should always have access to the nested context's active
         //      document URL through the browsing context.
         if let Some(ref load_data) = load_data {
-            *load_blocker = Some(LoadBlocker::new(&*document, LoadType::Subframe(load_data.url.clone())));
+            match load_data.js_eval_result {
+                Some(JsEvalResult::NoContent) => (),
+                _ => {
+                    *load_blocker = Some(LoadBlocker::new(&*document, LoadType::Subframe(load_data.url.clone())));
+                }
+            };
         }
 
         let window = window_from_node(self);

components/script/script_thread.rs

@@ -89,8 +89,8 @@ use script_runtime::{CommonScriptMsg, ScriptChan, ScriptThreadEventCategory};
 use script_runtime::{ScriptPort, StackRootTLS, get_reports, new_rt_and_cx};
 use script_traits::{CompositorEvent, ConstellationControlMsg, PaintMetricType};
 use script_traits::{DocumentActivity, DiscardBrowsingContext, EventResult};
-use script_traits::{InitialScriptState, LayoutMsg, LoadData, MouseButton, MouseEventType, MozBrowserEvent};
+use script_traits::{InitialScriptState, JsEvalResult, LayoutMsg, LoadData, MouseButton, MouseEventType};
-use script_traits::{NewLayoutInfo, ScriptToConstellationChan, ScriptMsg, UpdatePipelineIdReason};
+use script_traits::{MozBrowserEvent, NewLayoutInfo, ScriptToConstellationChan, ScriptMsg, UpdatePipelineIdReason};
 use script_traits::{ScriptThreadFactory, TimerEvent, TimerSchedulerMsg, TimerSource};
 use script_traits::{TouchEventType, TouchId, UntrustedNodeAddress, WindowSizeData, WindowSizeType};
 use script_traits::CompositorEvent::{KeyEvent, MouseButtonEvent, MouseMoveEvent, ResizeEvent};
@@ -122,6 +122,7 @@ use task_source::performance_timeline::PerformanceTimelineTaskSource;
 use task_source::user_interaction::UserInteractionTaskSource;
 use time::{get_time, precise_time_ns, Tm};
 use url::Position;
+use url::percent_encoding::percent_decode;
 use webdriver_handlers;
 use webvr_traits::{WebVREvent, WebVRMsg};
 
@@ -1509,7 +1510,7 @@ impl ScriptThread {
                                            load_data.url.clone(),
                                            origin);
         if load_data.url.as_str() == "about:blank" {
-            self.start_page_load_about_blank(new_load);
+            self.start_page_load_about_blank(new_load, load_data.js_eval_result);
         } else {
             self.pre_page_load(new_load, load_data);
         }
@@ -1679,6 +1680,18 @@ impl ScriptThread {
         // the pipeline exited before the page load completed.
         match idx {
             Some(idx) => {
+                // https://html.spec.whatwg.org/multipage/#process-a-navigate-response
+                // 2. If response's status is 204 or 205, then abort these steps.
+                match metadata {
+                    Some(Metadata { status: Some((204 ... 205, _)), .. }) => {
+                        self.script_sender
+                            .send((id.clone(), ScriptMsg::AbortLoadUrl))
+                            .unwrap();
+                        return None;
+                    },
+                    _ => ()
+                };
+
                 let load = self.incomplete_loads.borrow_mut().remove(idx);
                 metadata.map(|meta| self.load(meta, load))
             }
@@ -2118,39 +2131,7 @@ impl ScriptThread {
         // Notify devtools that a new script global exists.
         self.notify_devtools(document.Title(), final_url.clone(), (incomplete.pipeline_id, None));
 
-        let is_javascript = incomplete.url.scheme() == "javascript";
+        let parse_input = DOMString::new();
-        let parse_input = if is_javascript {
-            use url::percent_encoding::percent_decode;
-
-            // Turn javascript: URL into JS code to eval, according to the steps in
-            // https://html.spec.whatwg.org/multipage/#javascript-protocol
-
-            // This slice of the URL’s serialization is equivalent to (5.) to (7.):
-            // Start with the scheme data of the parsed URL;
-            // append question mark and query component, if any;
-            // append number sign and fragment component if any.
-            let encoded = &incomplete.url[Position::BeforePath..];
-
-            // Percent-decode (8.) and UTF-8 decode (9.)
-            let script_source = percent_decode(encoded.as_bytes()).decode_utf8_lossy();
-
-            // Script source is ready to be evaluated (11.)
-            unsafe {
-                let _ac = JSAutoCompartment::new(self.get_cx(), window.reflector().get_jsobject().get());
-                rooted!(in(self.get_cx()) let mut jsval = UndefinedValue());
-                window.upcast::<GlobalScope>().evaluate_js_on_global_with_result(
-                    &script_source, jsval.handle_mut());
-                let strval = DOMString::from_jsval(self.get_cx(),
-                                                   jsval.handle(),
-                                                   StringificationBehavior::Empty);
-                match strval {
-                    Ok(ConversionResult::Success(s)) => s,
-                    _ => DOMString::new(),
-                }
-            }
-        } else {
-            DOMString::new()
-        };
 
         document.set_https_state(metadata.https_state);
 
@@ -2329,8 +2310,16 @@ impl ScriptThread {
     /// for the given pipeline (specifically the "navigate" algorithm).
     fn handle_navigate(&self, parent_pipeline_id: PipelineId,
                               browsing_context_id: Option<BrowsingContextId>,
-                              load_data: LoadData,
+                              mut load_data: LoadData,
                               replace: bool) {
+        let is_javascript = load_data.url.scheme() == "javascript";
+        if is_javascript {
+            let window = self.documents.borrow().find_window(parent_pipeline_id);
+            if let Some(window) = window {
+                ScriptThread::eval_js_url(window.upcast::<GlobalScope>(), &mut load_data);
+            }
+        }
+
         match browsing_context_id {
             Some(browsing_context_id) => {
                 let iframe = self.documents.borrow().find_iframe(parent_pipeline_id, browsing_context_id);
@@ -2346,6 +2335,43 @@ impl ScriptThread {
         }
     }
 
+    pub fn eval_js_url(global_scope: &GlobalScope, load_data: &mut LoadData) {
+        // Turn javascript: URL into JS code to eval, according to the steps in
+        // https://html.spec.whatwg.org/multipage/#javascript-protocol
+
+        // This slice of the URL’s serialization is equivalent to (5.) to (7.):
+        // Start with the scheme data of the parsed URL;
+        // append question mark and query component, if any;
+        // append number sign and fragment component if any.
+        let encoded = &load_data.url.clone()[Position::BeforePath..];
+
+        // Percent-decode (8.) and UTF-8 decode (9.)
+        let script_source = percent_decode(encoded.as_bytes()).decode_utf8_lossy();
+
+        // Script source is ready to be evaluated (11.)
+        let _ac = JSAutoCompartment::new(global_scope.get_cx(), global_scope.reflector().get_jsobject().get());
+        rooted!(in(global_scope.get_cx()) let mut jsval = UndefinedValue());
+        global_scope.evaluate_js_on_global_with_result(&script_source, jsval.handle_mut());
+
+        load_data.js_eval_result = if jsval.get().is_string() {
+            unsafe {
+                let strval = DOMString::from_jsval(global_scope.get_cx(),
+                                                   jsval.handle(),
+                                                   StringificationBehavior::Empty);
+                match strval {
+                    Ok(ConversionResult::Success(s)) => {
+                        Some(JsEvalResult::Ok(String::from(s).as_bytes().to_vec()))
+                    },
+                    _ => None,
+                }
+            }
+        } else {
+            Some(JsEvalResult::NoContent)
+        };
+
+        load_data.url = ServoUrl::parse("about:blank").unwrap();
+    }
+
     fn handle_resize_event(&self, pipeline_id: PipelineId, new_size: WindowSizeData, size_type: WindowSizeType) {
         let document = match { self.documents.borrow().find_document(pipeline_id) } {
             Some(document) => document,
@@ -2377,7 +2403,7 @@ impl ScriptThread {
     /// argument until a notification is received that the fetch is complete.
     fn pre_page_load(&self, incomplete: InProgressLoad, load_data: LoadData) {
         let id = incomplete.pipeline_id.clone();
-        let mut req_init = RequestInit {
+        let req_init = RequestInit {
             url: load_data.url.clone(),
             method: load_data.method,
             destination: Destination::Document,
@@ -2393,10 +2419,6 @@ impl ScriptThread {
             .. RequestInit::default()
         };
 
-        if req_init.url.scheme() == "javascript" {
-            req_init.url = ServoUrl::parse("about:blank").unwrap();
-        }
-
         let context = ParserContext::new(id, load_data.url);
         self.incomplete_parser_contexts.borrow_mut().push((id, context));
 
@@ -2436,7 +2458,7 @@ impl ScriptThread {
 
     /// Synchronously fetch `about:blank`. Stores the `InProgressLoad`
     /// argument until a notification is received that the fetch is complete.
-    fn start_page_load_about_blank(&self, incomplete: InProgressLoad) {
+    fn start_page_load_about_blank(&self, incomplete: InProgressLoad, js_eval_result: Option<JsEvalResult>) {
         let id = incomplete.pipeline_id;
 
         self.incomplete_loads.borrow_mut().push(incomplete);
@@ -2446,8 +2468,20 @@ impl ScriptThread {
 
         let mut meta = Metadata::default(url);
         meta.set_content_type(Some(&mime!(Text / Html)));
+
+        // If this page load is the result of a javascript scheme url, map
+        // the evaluation result into a response.
+        let chunk = match js_eval_result {
+            Some(JsEvalResult::Ok(content)) => content,
+            Some(JsEvalResult::NoContent) => {
+                meta.status = Some((204, b"No Content".to_vec()));
+                vec![]
+            },
+            None => vec![]
+        };
+
         context.process_response(Ok(FetchMetadata::Unfiltered(meta)));
-        context.process_response_chunk(vec![]);
+        context.process_response_chunk(chunk);
         context.process_response_eof(Ok(()));
     }
 

components/script_traits/lib.rs

@@ -146,12 +146,24 @@ pub struct LoadData {
     pub headers: Headers,
     /// The data.
     pub data: Option<Vec<u8>>,
+    /// The result of evaluating a javascript scheme url.
+    pub js_eval_result: Option<JsEvalResult>,
     /// The referrer policy.
     pub referrer_policy: Option<ReferrerPolicy>,
     /// The referrer URL.
     pub referrer_url: Option<ServoUrl>,
 }
 
+/// The result of evaluating a javascript scheme url.
+#[derive(Clone, Debug, Deserialize, Serialize)]
+pub enum JsEvalResult {
+    /// The js evaluation had a non-string result, 204 status code.
+    /// https://html.spec.whatwg.org/multipage/#navigate 12.11
+    NoContent,
+    /// The js evaluation had a string result.
+    Ok(Vec<u8>)
+}
+
 impl LoadData {
     /// Create a new `LoadData` object.
     pub fn new(url: ServoUrl,
@@ -165,6 +177,7 @@ impl LoadData {
             method: Method::Get,
             headers: Headers::new(),
             data: None,
+            js_eval_result: None,
             referrer_policy: referrer_policy,
             referrer_url: referrer_url,
         }

components/script_traits/script_msg.rs

@@ -99,6 +99,8 @@ pub enum ScriptMsg {
     /// A new load has been requested, with an option to replace the current entry once loaded
     /// instead of adding a new entry.
     LoadUrl(LoadData, bool),
+    /// Abort loading after sending a LoadUrl message.
+    AbortLoadUrl,
     /// Post a message to the currently active window of a given browsing context.
     PostMessage(BrowsingContextId, Option<ImmutableOrigin>, Vec<u8>),
     /// Dispatch a mozbrowser event to the parent of a mozbrowser iframe.

tests/wpt/metadata/MANIFEST.json

@@ -326433,6 +326433,12 @@
      {}
     ]
    ],
+   "html/browsers/browsing-the-web/navigating-across-documents/javascript-url-global-scope.html": [
+    [
+     "/html/browsers/browsing-the-web/navigating-across-documents/javascript-url-global-scope.html",
+     {}
+    ]
+   ],
    "html/browsers/browsing-the-web/navigating-across-documents/javascript-url-query-fragment-components.html": [
     [
      "/html/browsers/browsing-the-web/navigating-across-documents/javascript-url-query-fragment-components.html",
@@ -559898,6 +559904,10 @@
    "3842ac825b9fb33d0d95ef99f77c8c7d02a88e9a",
    "support"
   ],
+  "html/browsers/browsing-the-web/navigating-across-documents/javascript-url-global-scope.html": [
+   "d678c54e2c20d5f240fd68790ea4e03512db2c8a",
+   "testharness"
+  ],
   "html/browsers/browsing-the-web/navigating-across-documents/javascript-url-query-fragment-components.html": [
    "1278f37be250f761f84bf352ebff8ed4c8a04e96",
    "testharness"

tests/wpt/metadata/XMLHttpRequest/open-url-javascript-window-2.htm.ini

@@ -1,6 +1,5 @@
 [open-url-javascript-window-2.htm]
   type: testharness
-  expected: TIMEOUT
   [XMLHttpRequest: open() - resolving URLs (javascript: <iframe>; 2)]
-    expected: TIMEOUT
+    expected: FAIL
 

tests/wpt/metadata/XMLHttpRequest/open-url-javascript-window.htm.ini

@@ -1,6 +0,0 @@
-[open-url-javascript-window.htm]
-  type: testharness
-  expected: TIMEOUT
-  [XMLHttpRequest: open() - resolving URLs (javascript: <iframe>; 1)]
-    expected: TIMEOUT
-

tests/wpt/metadata/dom/nodes/Document-contentType/contentType/contenttype_javascripturi.html.ini

@@ -1,5 +0,0 @@
-[contenttype_javascripturi.html]
-  type: testharness
-  [Javascript URI document.contentType === 'text/html']
-    expected: FAIL
-

tests/wpt/metadata/html/browsers/browsing-the-web/navigating-across-documents/013.html.ini

@@ -1,5 +0,0 @@
-[013.html]
-  type: testharness
-  [Link with onclick navigation to javascript url with delayed document.write and href navigation ]
-    expected: FAIL
-

tests/wpt/metadata/html/browsers/browsing-the-web/navigating-across-documents/014.html.ini

@@ -1,5 +0,0 @@
-[014.html]
-  type: testharness
-  [ Link with javascript onclick form submission script order ]
-    expected: FAIL
-

tests/wpt/metadata/html/browsers/browsing-the-web/navigating-across-documents/015.html.ini

@@ -1,5 +0,0 @@
-[015.html]
-  type: testharness
-  [ Link with javascript onclick and href script order ]
-    expected: FAIL
-

tests/wpt/metadata/html/browsers/browsing-the-web/navigating-across-documents/javascript-url-query-fragment-components.html.ini

@@ -1,5 +0,0 @@
-[javascript-url-query-fragment-components.html]
-  type: testharness
-  [iframes with javascript src]
-    expected: FAIL
-

tests/wpt/metadata/html/dom/documents/dom-tree-accessors/Document.currentScript.html.ini

@@ -7,6 +7,3 @@
   [Script script-svg]
     expected: NOTRUN
 
-  [Script iframe-src]
-    expected: NOTRUN
-

tests/wpt/metadata/old-tests/submission/Opera/script_scheduling/028.html.ini

@@ -1,5 +0,0 @@
-[028.html]
-  type: testharness
-  [ scheduler: javascript: URL]
-    expected: FAIL
-

tests/wpt/metadata/old-tests/submission/Opera/script_scheduling/029.html.ini

@@ -1,5 +0,0 @@
-[029.html]
-  type: testharness
-  [ scheduler: javascript: URL in HREF]
-    expected: FAIL
-

tests/wpt/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/javascript-url-global-scope.html

@@ -0,0 +1,16 @@
+<!doctype html>
+<meta charset=utf-8>
+<script src=/resources/testharness.js></script>
+<script src=/resources/testharnessreport.js></script>
+
+<a id="javascript-link" href="javascript:changeStatus()">link</a>
+
+<script>
+function changeStatus() {
+  t.done();
+}
+
+var t = async_test(function(t) {
+  document.querySelector("#javascript-link").click();
+}, "javascript: scheme urls should be executed in current global scope");
+</script>