Bug 1403923 - Safely shutdown Firefox from in delete_session. r=jgraham

With the request to shutdown the browser, a given amount of time
has to be waited to allow the process to shutdown itself. Only
if the process is still running afterward it has to be killed.

Firefox has an integrated background monitor which observes
long running threads during shutdown, and kills those after
65s. To allow Firefox to shutdown on its own, geckodriver
has to wait that time, and some additional seconds.

MozReview-Commit-ID: 4LRLQE0jZzw

--HG--
extra : rebase_source : c33c163d4d06768ea6616b97a25f986f5ea37e5d

testing/geckodriver/CHANGES.md

@@ -14,6 +14,9 @@ Unreleased
 - Backtraces from geckodriver no longer substitute for missing
   Marionette stacktraces
 
+- `Delete Session` now allows Firefox to safely shutdown within 130s before
+  force-killing the process
+
 
 0.19.1 (2017-10-30)
 -------------------

testing/geckodriver/src/marionette.rs

@@ -57,6 +57,10 @@ use prefs;
 
 const DEFAULT_HOST: &'static str = "localhost";
 
+// Firefox' integrated background monitor which observes long running threads during
+// shutdown kills those after 65s. Wait some additional seconds for a safe shutdown
+const TIMEOUT_BROWSER_SHUTDOWN: u64 = 70 * 1000;
+
 pub fn extension_routes() -> Vec<(Method, &'static str, GeckoExtensionRoute)> {
     return vec![(Method::Get, "/session/{sessionId}/moz/context", GeckoExtensionRoute::GetContext),
              (Method::Post, "/session/{sessionId}/moz/context", GeckoExtensionRoute::SetContext),
@@ -573,18 +577,51 @@ impl WebDriverHandler<GeckoExtensionRoute> for MarionetteHandler {
         }
     }
 
-    fn delete_session(&mut self, _: &Option<Session>) {
-        if let Ok(connection) = self.connection.lock() {
-            if let Some(ref conn) = *connection {
+    fn delete_session(&mut self, session: &Option<Session>) {
+        // If there is still an active session send a delete session command
+        // and wait for the browser to quit
+        if let Some(ref s) = *session {
+            let delete_session = WebDriverMessage {
+                session_id: Some(s.id.clone()),
+                command: WebDriverCommand::DeleteSession
+            };
+            let _ = self.handle_command(session, delete_session);
+
+            if let Some(ref mut runner) = self.browser {
+                let timeout = TIMEOUT_BROWSER_SHUTDOWN;
+                let poll_interval = 100;
+                let poll_attempts = timeout / poll_interval;
+                let mut poll_attempt = 0;
+
+                while runner.is_running() {
+                    if poll_attempt <= poll_attempts {
+                        debug!("Waiting for the browser process to shutdown");
+                        poll_attempt += 1;
+                        sleep(Duration::from_millis(poll_interval));
+                    } else {
+                        warn!("Browser process did not shutdown");
+                        break;
+                    }
+                }
+            }
+        }
+
+        if let Ok(ref mut connection) = self.connection.lock() {
+            if let Some(conn) = connection.as_mut() {
                 conn.close();
             }
         }
+
+        // If the browser is still open then kill the process
         if let Some(ref mut runner) = self.browser {
-            debug!("Stopping browser process");
-            if runner.stop().is_err() {
-                error!("Failed to kill browser process");
-            };
+            if runner.is_running() {
+                info!("Forcing a shutdown of the browser process");
+                if runner.stop().is_err() {
+                    error!("Failed to kill browser process");
+                };
+            }
         }
+
         self.connection = Mutex::new(None);
         self.browser = None;
     }

testing/web-platform/meta/webdriver/tests/execute_async_script/user_prompts.py.ini

@@ -1,4 +1,7 @@
 [user_prompts.py]
+  expected:
+    if debug: TIMEOUT # https://bugzilla.mozilla.org/show_bug.cgi?id=1425559
+
   [user_prompts.py::test_handle_prompt_accept]
     expected: FAIL
 

testing/web-platform/meta/webdriver/tests/execute_script/user_prompts.py.ini

@@ -1,4 +1,7 @@
 [user_prompts.py]
+  expected:
+    if debug: TIMEOUT # https://bugzilla.mozilla.org/show_bug.cgi?id=1425559
+
   [user_prompts.py::test_handle_prompt_accept]
     expected: FAIL
 

testing/web-platform/tests/webdriver/tests/state/get_element_attribute.py

@@ -1,3 +1,5 @@
+# META: timeout=long
+
 import pytest
 
 from tests.support.asserts import assert_error, assert_success, assert_dialog_handled

testing/web-platform/tests/webdriver/tests/state/get_element_property.py

@@ -1,3 +1,5 @@
+# META: timeout=long
+
 from tests.support.asserts import assert_error, assert_dialog_handled, assert_success
 from tests.support.inline import inline
 from tests.support.fixtures import create_dialog

testing/web-platform/tests/webdriver/tests/state/get_element_tag_name.py

@@ -1,3 +1,5 @@
+# META: timeout=long
+
 from tests.support.asserts import assert_error, assert_dialog_handled, assert_success
 from tests.support.inline import inline
 from tests.support.fixtures import create_dialog

testing/web-platform/tests/webdriver/tests/state/is_element_selected.py

@@ -1,3 +1,5 @@
+# META: timeout=long
+
 from tests.support.asserts import assert_error, assert_dialog_handled, assert_success
 from tests.support.inline import inline
 from tests.support.fixtures import create_dialog

testing/webdriver/src/server.rs

@@ -25,7 +25,7 @@ enum DispatchMessage<U: WebDriverExtensionRoute> {
 
 #[derive(Clone, Debug, PartialEq)]
 pub struct Session {
-    id: String
+    pub id: String
 }
 
 impl Session {