improve spec compliance of window.close

12 files changed, 123 insertions, 39 deletions

diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs
index a46f436147b..6275b97bb6a 100644
--- a/components/compositing/compositor.rs
+++ b/components/compositing/compositor.rs
@@ -385,10 +385,6 @@ impl<Window: WindowMethods> IOCompositor<Window> {
                 return false
             }
 
-            (Msg::Exit, _) => {
-                self.start_shutting_down();
-            }
-
             (Msg::ShutdownComplete, _) => {
                 self.finish_shutting_down();
                 return false;
diff --git a/components/compositing/compositor_thread.rs b/components/compositing/compositor_thread.rs
index 76b3172b5dd..15fd4d31cd6 100644
--- a/components/compositing/compositor_thread.rs
+++ b/components/compositing/compositor_thread.rs
@@ -68,14 +68,10 @@ impl CompositorProxy {
 
 /// Messages from the painting thread and the constellation thread to the compositor thread.
 pub enum Msg {
-    /// Requests that the compositor shut down.
-    Exit,
-
     /// Informs the compositor that the constellation has completed shutdown.
     /// Required because the constellation can have pending calls to make
     /// (e.g. SetFrameTree) at the time that we send it an ExitMsg.
     ShutdownComplete,
-
     /// Alerts the compositor that the given pipeline has changed whether it is running animations.
     ChangeRunningAnimationsState(PipelineId, AnimationState),
     /// Replaces the current frame tree, typically called during main frame navigation.
@@ -123,7 +119,6 @@ pub enum Msg {
 impl Debug for Msg {
     fn fmt(&self, f: &mut Formatter) -> Result<(), Error> {
         match *self {
-            Msg::Exit => write!(f, "Exit"),
             Msg::ShutdownComplete => write!(f, "ShutdownComplete"),
             Msg::ChangeRunningAnimationsState(..) => write!(f, "ChangeRunningAnimationsState"),
             Msg::SetFrameTree(..) => write!(f, "SetFrameTree"),
diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs
index 2da318fef7d..e1ecb766c47 100644
--- a/components/constellation/constellation.rs
+++ b/components/constellation/constellation.rs
@@ -1034,6 +1034,10 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             FromScriptMsg::DiscardDocument => {
                 self.handle_discard_document(source_top_ctx_id, source_pipeline_id);
             }
+            FromScriptMsg::DiscardTopLevelBrowsingContext => {
+                self.handle_close_top_level_browsing_context(source_top_ctx_id);
+            }
+
             FromScriptMsg::InitiateNavigateRequest(req_init, cancel_chan) => {
                 self.handle_navigate_request(source_pipeline_id, req_init, cancel_chan);
             }
@@ -1142,9 +1146,6 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             FromScriptMsg::GetScreenAvailSize(send) => {
                 self.compositor_proxy.send(ToCompositorMsg::GetScreenAvailSize(send));
             }
-            FromScriptMsg::Exit => {
-                self.compositor_proxy.send(ToCompositorMsg::Exit);
-            }
             FromScriptMsg::LogEntry(thread_name, entry) => {
                 self.handle_log_entry(Some(source_top_ctx_id), thread_name, entry);
             }
diff --git a/components/embedder_traits/lib.rs b/components/embedder_traits/lib.rs
index 9122a76f0ef..b1698c95728 100644
--- a/components/embedder_traits/lib.rs
+++ b/components/embedder_traits/lib.rs
@@ -102,6 +102,8 @@ pub enum EmbedderMsg {
     LoadStart,
     /// The load of a page has completed
     LoadComplete,
+    /// A browser is to be closed
+    CloseBrowser,
     /// A pipeline panicked. First string is the reason, second one is the backtrace.
     Panic(String, Option<String>),
     /// Open dialog to select bluetooth device.
@@ -130,6 +132,7 @@ impl Debug for EmbedderMsg {
             EmbedderMsg::SetCursor(..) => write!(f, "SetCursor"),
             EmbedderMsg::NewFavicon(..) => write!(f, "NewFavicon"),
             EmbedderMsg::HeadParsed => write!(f, "HeadParsed"),
+            EmbedderMsg::CloseBrowser => write!(f, "CloseBrowser"),
             EmbedderMsg::HistoryChanged(..) => write!(f, "HistoryChanged"),
             EmbedderMsg::SetFullscreenState(..) => write!(f, "SetFullscreenState"),
             EmbedderMsg::LoadStart => write!(f, "LoadStart"),
diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs
index 3a24a1ac95d..756bfb5a555 100644
--- a/components/script/dom/window.rs
+++ b/components/script/dom/window.rs
@@ -11,6 +11,7 @@ use devtools_traits::{ScriptToDevtoolsControlMsg, TimelineMarker, TimelineMarker
 use dom::bindings::cell::DomRefCell;
 use dom::bindings::codegen::Bindings::DocumentBinding::{DocumentMethods, DocumentReadyState};
 use dom::bindings::codegen::Bindings::FunctionBinding::Function;
+use dom::bindings::codegen::Bindings::HistoryBinding::HistoryBinding::HistoryMethods;
 use dom::bindings::codegen::Bindings::PermissionStatusBinding::PermissionState;
 use dom::bindings::codegen::Bindings::RequestBinding::RequestInit;
 use dom::bindings::codegen::Bindings::WindowBinding::{self, FrameRequestCallback, WindowMethods};
@@ -543,9 +544,27 @@ impl WindowMethods for Window {
 
     // https://html.spec.whatwg.org/multipage/#dom-window-close
     fn Close(&self) {
-        self.main_thread_script_chan()
-            .send(MainThreadScriptMsg::ExitWindow(self.upcast::<GlobalScope>().pipeline_id()))
-            .unwrap();
+        // Note: check the length of the "session history", as opposed to the joint session history?
+        // see https://github.com/whatwg/html/issues/3734
+        if let Ok(history_length) = self.History().GetLength() {
+            // TODO: allow auxilliary browsing contexts created by script to be script-closeable,
+            // regardless of history length.
+            // https://html.spec.whatwg.org/multipage/#script-closable
+            let is_script_closable = self.is_top_level() && history_length == 1;
+            if is_script_closable {
+                let doc = self.Document();
+                // https://html.spec.whatwg.org/multipage/#closing-browsing-contexts
+                // Step 1, prompt to unload.
+                if doc.prompt_to_unload(false) {
+                    // Step 2, unload.
+                    doc.unload(false, false);
+                    // Step 3, remove from the user interface
+                    let _ = self.send_to_embedder(EmbedderMsg::CloseBrowser);
+                    // Step 4, discard browsing context.
+                    let _ = self.send_to_constellation(ScriptMsg::DiscardTopLevelBrowsingContext);
+                }
+            }
+        }
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-document-2
diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs
index a28bdfbb5bf..1852e467189 100644
--- a/components/script/script_thread.rs
+++ b/components/script/script_thread.rs
@@ -220,9 +220,6 @@ enum MixedMessage {
 pub enum MainThreadScriptMsg {
     /// Common variants associated with the script messages
     Common(CommonScriptMsg),
-    /// Notifies the script that a window associated with a particular pipeline
-    /// should be closed (only dispatched to ScriptThread).
-    ExitWindow(PipelineId),
     /// Begins a content-initiated load on the specified pipeline (only
     /// dispatched to ScriptThread). Allows for a replace bool to be passed. If true,
     /// the current entry will be replaced instead of a new entry being added.
@@ -1193,7 +1190,6 @@ impl ScriptThread {
                     MainThreadScriptMsg::Common(CommonScriptMsg::Task(_, _, pipeline_id)) =>
                         pipeline_id,
                     MainThreadScriptMsg::Common(CommonScriptMsg::CollectReports(_)) => None,
-                    MainThreadScriptMsg::ExitWindow(pipeline_id) => Some(pipeline_id),
                     MainThreadScriptMsg::Navigate(pipeline_id, ..) => Some(pipeline_id),
                     MainThreadScriptMsg::WorkletLoaded(pipeline_id) => Some(pipeline_id),
                     MainThreadScriptMsg::RegisterPaintWorklet { pipeline_id, .. } => Some(pipeline_id),
@@ -1344,9 +1340,6 @@ impl ScriptThread {
             MainThreadScriptMsg::Navigate(parent_pipeline_id, load_data, replace) => {
                 self.handle_navigate(parent_pipeline_id, None, load_data, replace)
             },
-            MainThreadScriptMsg::ExitWindow(id) => {
-                self.handle_exit_window_msg(id)
-            },
             MainThreadScriptMsg::Common(CommonScriptMsg::Task(_, task, _)) => {
                 task.run_box()
             }
@@ -1715,20 +1708,6 @@ impl ScriptThread {
         window.set_window_size(new_size);
     }
 
-    /// We have gotten a window.close from script, which we pass on to the compositor.
-    /// We do not shut down the script thread now, because the compositor will ask the
-    /// constellation to shut down the pipeline, which will clean everything up
-    /// normally. If we do exit, we will tear down the DOM nodes, possibly at a point
-    /// where layout is still accessing them.
-    fn handle_exit_window_msg(&self, id: PipelineId) {
-        debug!("script thread handling exit window msg");
-
-        // TODO(tkuehn): currently there is only one window,
-        // so this can afford to be naive and just shut down the
-        // constellation. In the future it'll need to be smarter.
-        self.script_sender.send((id, ScriptMsg::Exit)).unwrap();
-    }
-
     /// We have received notification that the response associated with a load has completed.
     /// Kick off the document and frame tree creation process using the result.
     fn handle_page_headers_available(&self, id: &PipelineId,
diff --git a/components/script_traits/script_msg.rs b/components/script_traits/script_msg.rs
index c9cb879f1c2..4487cb10d5d 100644
--- a/components/script_traits/script_msg.rs
+++ b/components/script_traits/script_msg.rs
@@ -149,6 +149,8 @@ pub enum ScriptMsg {
     LogEntry(Option<String>, LogEntry),
     /// Discard the document.
     DiscardDocument,
+    /// Discard the browsing context.
+    DiscardTopLevelBrowsingContext,
     /// Notifies the constellation that this pipeline has exited.
     PipelineExited,
     /// Send messages from postMessage calls from serviceworker
@@ -162,8 +164,6 @@ pub enum ScriptMsg {
     GetScreenSize(IpcSender<(DeviceUintSize)>),
     /// Get the available screen size (pixel)
     GetScreenAvailSize(IpcSender<(DeviceUintSize)>),
-    /// Requests that the compositor shut down.
-    Exit,
 }
 
 impl fmt::Debug for ScriptMsg {
@@ -200,13 +200,13 @@ impl fmt::Debug for ScriptMsg {
             TouchEventProcessed(..) => "TouchEventProcessed",
             LogEntry(..) => "LogEntry",
             DiscardDocument => "DiscardDocument",
+            DiscardTopLevelBrowsingContext => "DiscardTopLevelBrowsingContext",
             PipelineExited => "PipelineExited",
             ForwardDOMMessage(..) => "ForwardDOMMessage",
             RegisterServiceWorker(..) => "RegisterServiceWorker",
             GetClientWindow(..) => "GetClientWindow",
             GetScreenSize(..) => "GetScreenSize",
             GetScreenAvailSize(..) => "GetScreenAvailSize",
-            Exit => "Exit",
         };
         write!(formatter, "ScriptMsg::{}", variant)
     }
diff --git a/ports/servo/browser.rs b/ports/servo/browser.rs
index 7ecefcc730f..17e605d8259 100644
--- a/ports/servo/browser.rs
+++ b/ports/servo/browser.rs
@@ -302,6 +302,12 @@ impl Browser {
                 EmbedderMsg::LoadComplete => {
                     self.loading_state = Some(LoadingState::Loaded);
                 }
+                EmbedderMsg::CloseBrowser => {
+                    self.browser_id = None;
+                    // Nothing left to do for now,
+                    // but could hide a tab, and show another one, instead of quitting.
+                    self.event_queue.push(WindowEvent::Quit);
+                },
                 EmbedderMsg::Shutdown => {
                     self.shutdown_requested = true;
                 },
diff --git a/tests/wpt/metadata/MANIFEST.json b/tests/wpt/metadata/MANIFEST.json
index 3e5f358c817..6f6cdeb2bdc 100644
--- a/tests/wpt/metadata/MANIFEST.json
+++ b/tests/wpt/metadata/MANIFEST.json
@@ -279024,6 +279024,11 @@
      {}
     ]
    ],
+   "html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-uncloseable-1.html": [
+    [
+     {}
+    ]
+   ],
    "html/browsers/browsing-the-web/unloading-documents/prompt/001-1.html": [
     [
      {}
@@ -334998,6 +335003,18 @@
      {}
     ]
    ],
+   "html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-closeable.html": [
+    [
+     "/html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-closeable.html",
+     {}
+    ]
+   ],
+   "html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-uncloseable.html": [
+    [
+     "/html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-uncloseable.html",
+     {}
+    ]
+   ],
    "html/browsers/browsing-the-web/unloading-documents/prompt/001.html": [
     [
      "/html/browsers/browsing-the-web/unloading-documents/prompt/001.html",
@@ -566859,6 +566876,18 @@
    "2b3a56895dbe6450ed38ebbb31a915c9e8b7abd6",
    "testharness"
   ],
+  "html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-closeable.html": [
+   "7b60961703fd447aff290aa5fedf6950b242b9d5",
+   "testharness"
+  ],
+  "html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-uncloseable-1.html": [
+   "0e8cf55368a34a0367763cdf902fdf6a5dc51f28",
+   "support"
+  ],
+  "html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-uncloseable.html": [
+   "faa25d4925073f71b3ee451427b253cc232c01f7",
+   "testharness"
+  ],
   "html/browsers/browsing-the-web/unloading-documents/prompt/001-1.html": [
    "758c04e8df6778e435346c59280c17e02295cca6",
    "support"
diff --git a/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-closeable.html b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-closeable.html
new file mode 100644
index 00000000000..0c11b587eb1
--- /dev/null
+++ b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-closeable.html
@@ -0,0 +1,22 @@
+<!doctype html>
+<title>beforeunload and unload events fire after window.close() in script-closeable browsing context</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div id="log"></div>
+<script>
+beforeunload_fired = false;
+var t = async_test();
+
+onload = t.step_func(function() {
+  window.close();
+});
+
+onbeforeunload = t.step_func(function() {
+  beforeunload_fired = true;
+});
+
+onunload = t.step_func(function() {
+  assert_true(beforeunload_fired);
+  t.done()
+});
+</script>
diff --git a/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-uncloseable-1.html b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-uncloseable-1.html
new file mode 100644
index 00000000000..3a557ce34ea
--- /dev/null
+++ b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-uncloseable-1.html
@@ -0,0 +1,10 @@
+<!doctype html>
+script-uncloseable-1
+<script>
+onbeforeunload = function() {
+  parent.beforeunload_fired = true;
+};
+onunload = function() {
+  parent.unload_fired = true;
+};
+</script>
diff --git a/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-uncloseable.html b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-uncloseable.html
new file mode 100644
index 00000000000..f6a17d740b5
--- /dev/null
+++ b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/unloading-documents/prompt-and-unload-script-uncloseable.html
@@ -0,0 +1,24 @@
+<!doctype html>
+<title>beforeunload and unload events do not fire after window.close() in script-uncloseable browsing context</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div id="log"></div>
+<script>
+var beforeunload_fired = false;
+var unload_fired = false;
+var t = async_test();
+
+onload = t.step_func(function() {
+  var iframe = document.getElementsByTagName("iframe")[0]
+  iframe.onload = t.step_func(function() {
+    iframe.contentWindow.close()
+    t.step_timeout(function() {
+      assert_false(beforeunload_fired);
+      assert_false(unload_fired);
+      t.done();
+    }, 1000);
+  });
+  iframe.src = "prompt-and-unload-script-uncloseable-1.html";
+});
+</script>
+<iframe></iframe>