Merge autoland to mozilla-central. a=merge
authorarthur.iakab <aiakab@mozilla.com>
Sun, 11 Mar 2018 23:44:57 +0200
changeset 462565 6d0d32f6570d875ae9191c07be0355e9083114e8
parent 462556 0471c6ccf344ed039d0c514e254a7edbdb8a5905 (current diff)
parent 462564 a5d22f442915eaeca18002aa555aa22936f6c855 (diff)
child 462570 a6f5fb18e6bcc9bffe4a0209a22d8a25510936be
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmerge
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Merge autoland to mozilla-central. a=merge
--- a/servo/components/style/stylesheets/keyframes_rule.rs
+++ b/servo/components/style/stylesheets/keyframes_rule.rs
@@ -1,16 +1,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! Keyframes: https://drafts.csswg.org/css-animations/#keyframes
 
 use cssparser::{AtRuleParser, Parser, QualifiedRuleParser, RuleListParser, ParserInput, CowRcStr};
-use cssparser::{DeclarationListParser, DeclarationParser, parse_one_rule, SourceLocation};
+use cssparser::{DeclarationListParser, DeclarationParser, parse_one_rule, SourceLocation, Token};
 use error_reporting::{NullReporter, ContextualParseError, ParseErrorReporter};
 use parser::{ParserContext, ParserErrorContext};
 use properties::{DeclarationSource, Importance, PropertyDeclaration, PropertyDeclarationBlock, PropertyId};
 use properties::{PropertyDeclarationId, LonghandId, SourcePropertyDeclaration};
 use properties::LonghandIdSet;
 use properties::longhands::transition_timing_function::single_value::SpecifiedValue as SpecifiedTimingFunction;
 use servo_arc::Arc;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, SharedRwLock, SharedRwLockReadGuard, Locked, ToCssWithGuard};
@@ -121,30 +121,31 @@ impl KeyframePercentage {
     /// Trivially constructs a new `KeyframePercentage`.
     #[inline]
     pub fn new(value: f32) -> KeyframePercentage {
         debug_assert!(value >= 0. && value <= 1.);
         KeyframePercentage(value)
     }
 
     fn parse<'i, 't>(input: &mut Parser<'i, 't>) -> Result<KeyframePercentage, ParseError<'i>> {
-        let percentage = if input.try(|input| input.expect_ident_matching("from")).is_ok() {
-            KeyframePercentage::new(0.)
-        } else if input.try(|input| input.expect_ident_matching("to")).is_ok() {
-            KeyframePercentage::new(1.)
-        } else {
-            let percentage = input.expect_percentage()?;
-            if percentage >= 0. && percentage <= 1. {
-                KeyframePercentage::new(percentage)
-            } else {
-                return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-            }
-        };
-
-        Ok(percentage)
+        let token = input.next()?.clone();
+        match token {
+            Token::Ident(ref identifier) if identifier.as_ref().eq_ignore_ascii_case("from") => {
+                Ok(KeyframePercentage::new(0.))
+            },
+            Token::Ident(ref identifier) if identifier.as_ref().eq_ignore_ascii_case("to") => {
+                Ok(KeyframePercentage::new(1.))
+            },
+            Token::Percentage { unit_value: percentage, .. } if percentage >= 0. && percentage <= 1. => {
+                Ok(KeyframePercentage::new(percentage))
+            },
+            _ => {
+                Err(input.new_unexpected_token_error(token))
+            },
+        }
     }
 }
 
 /// A keyframes selector is a list of percentages or from/to symbols, which are
 /// converted at parse time to percentages.
 #[css(comma)]
 #[derive(Clone, Debug, Eq, PartialEq, ToCss)]
 pub struct KeyframeSelector(#[css(iterable)] Vec<KeyframePercentage>);
--- a/testing/geckodriver/src/marionette.rs
+++ b/testing/geckodriver/src/marionette.rs
@@ -12,18 +12,18 @@ use std::error::Error;
 use std::fs::File;
 use std::io::Error as IoError;
 use std::io::ErrorKind;
 use std::io::prelude::*;
 use std::path::PathBuf;
 use std::io::Result as IoResult;
 use std::net::{TcpListener, TcpStream};
 use std::sync::Mutex;
-use std::thread::sleep;
-use std::time::Duration;
+use std::thread;
+use std::time;
 use uuid::Uuid;
 use webdriver::capabilities::CapabilitiesMatching;
 use webdriver::command::{WebDriverCommand, WebDriverMessage, Parameters,
                          WebDriverExtensionCommand};
 use webdriver::command::WebDriverCommand::{
     NewSession, DeleteSession, Status, Get, GetCurrentUrl,
     GoBack, GoForward, Refresh, GetTitle, GetPageSource, GetWindowHandle,
     GetWindowHandles, CloseWindow, SetWindowRect, GetWindowRect,
@@ -51,20 +51,16 @@ use webdriver::server::{WebDriverHandler
 use webdriver::httpapi::{WebDriverExtensionRoute};
 
 use capabilities::{FirefoxCapabilities, FirefoxOptions};
 use logging;
 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),
              (Method::Post,
               "/session/{sessionId}/moz/xbl/{elementId}/anonymous_children",
               GeckoExtensionRoute::XblAnonymousChildren),
              (Method::Post,
               "/session/{sessionId}/moz/xbl/{elementId}/anonymous_by_attribute",
@@ -566,73 +562,51 @@ impl WebDriverHandler<GeckoExtensionRout
             Ok(ref mut connection) => {
                 match connection.as_mut() {
                     Some(conn) => {
                         conn.send_command(resolved_capabilities, &msg)
                             .map_err(|mut err| {
                                 // Shutdown the browser if no session can
                                 // be established due to errors.
                                 if let NewSession(_) = msg.command {
-                                    err.delete_session=true;
+                                    err.delete_session = true;
                                 }
                                 err})
                     },
                     None => panic!("Connection missing")
                 }
             },
             Err(_) => {
                 Err(WebDriverError::new(
                     ErrorStatus::UnknownError,
                     "Failed to aquire Marionette 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
+                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 {
-            if runner.is_running() {
-                info!("Forcing a shutdown of the browser process");
-                if runner.stop().is_err() {
-                    error!("Failed to kill browser process");
-                };
+            // TODO(https://bugzil.la/1443922):
+            // Use toolkit.asyncshutdown.crash_timout pref
+            if runner.wait(time::Duration::from_secs(70)).is_err() {
+                error!("Failed to stop browser process");
             }
         }
 
         self.connection = Mutex::new(None);
         self.browser = None;
     }
 }
 
@@ -980,20 +954,20 @@ impl MarionetteSession {
                                     |x| {
                                         Ok((try_opt!(x.as_string(),
                                                      ErrorStatus::UnknownError,
                                                      "Cookie domain must be string")).to_string())
                                     }));
             let expiry = try!(
                 Nullable::from_json(x.find("expiry").unwrap_or(&Json::Null),
                                     |x| {
-                                        Ok(Date::new((try_opt!(
+                                        Ok(Date::new(try_opt!(
                                             x.as_u64(),
                                             ErrorStatus::UnknownError,
-                                            "Cookie expiry must be a positive integer"))))
+                                            "Cookie expiry must be a positive integer")))
                                     }));
             let secure = try_opt!(
                 x.find("secure").map_or(Some(false), |x| x.as_boolean()),
                 ErrorStatus::UnknownError,
                 "Cookie secure flag must be boolean");
             let http_only = try_opt!(
                 x.find("httpOnly").map_or(Some(false), |x| x.as_boolean()),
                 ErrorStatus::UnknownError,
@@ -1336,64 +1310,69 @@ pub struct MarionetteConnection {
     pub session: MarionetteSession
 }
 
 impl MarionetteConnection {
     pub fn new(port: u16, session_id: Option<String>) -> MarionetteConnection {
         MarionetteConnection {
             port: port,
             stream: None,
-            session: MarionetteSession::new(session_id)
+            session: MarionetteSession::new(session_id),
         }
     }
 
     pub fn connect(&mut self, browser: &mut Option<FirefoxProcess>) -> WebDriverResult<()> {
-        let timeout = 60 * 1000;  // ms
-        let poll_interval = 100;  // ms
+        let timeout = 60 * 1000; // ms
+        let poll_interval = 100; // ms
         let poll_attempts = timeout / poll_interval;
         let mut poll_attempt = 0;
 
         loop {
-            // If the process is gone, immediately abort the connection attempts
+            // immediately abort connection attempts if process disappears
             if let &mut Some(ref mut runner) = browser {
-                let status = runner.status();
-                if status.is_err() || status.as_ref().map(|x| *x).unwrap_or(None) != None {
+                let exit_status = match runner.try_wait() {
+                    Ok(Some(status)) => Some(
+                        status
+                            .code()
+                            .map(|c| c.to_string())
+                            .unwrap_or("signal".into()),
+                    ),
+                    Ok(None) => None,
+                    Err(_) => Some("{unknown}".into()),
+                };
+                if let Some(s) = exit_status {
                     return Err(WebDriverError::new(
                         ErrorStatus::UnknownError,
-                        format!("Process unexpectedly closed with status: {}", status
-                            .ok()
-                            .and_then(|x| x)
-                            .and_then(|x| x.code())
-                            .map(|x| x.to_string())
-                            .unwrap_or("{unknown}".into()))));
+                        format!("Process unexpectedly closed with status {}", s),
+                    ));
                 }
             }
 
             match TcpStream::connect(&(DEFAULT_HOST, self.port)) {
                 Ok(stream) => {
                     self.stream = Some(stream);
-                    break
-                },
+                    break;
+                }
                 Err(e) => {
                     trace!("  connection attempt {}/{}", poll_attempt, poll_attempts);
                     if poll_attempt <= poll_attempts {
                         poll_attempt += 1;
-                        sleep(Duration::from_millis(poll_interval));
+                        thread::sleep(time::Duration::from_millis(poll_interval));
                     } else {
                         return Err(WebDriverError::new(
-                            ErrorStatus::UnknownError, e.description().to_owned()));
+                            ErrorStatus::UnknownError,
+                            e.description().to_owned(),
+                        ));
                     }
                 }
             }
-        };
+        }
 
         debug!("Connected to Marionette on {}:{}", DEFAULT_HOST, self.port);
-
-        try!(self.handshake());
-        Ok(())
+        self.handshake()
     }
 
     fn handshake(&mut self) -> WebDriverResult<()> {
         let resp = try!(self.read_resp());
         let handshake_data = try!(Json::from_str(&*resp));
 
         let data = try_opt!(handshake_data.as_object(),
                             ErrorStatus::UnknownError,
--- a/testing/mozbase/rust/mozrunner/src/runner.rs
+++ b/testing/mozbase/rust/mozrunner/src/runner.rs
@@ -1,21 +1,23 @@
 use mozprofile::prefreader::PrefReaderError;
 use mozprofile::profile::Profile;
-use std::ascii::AsciiExt;
 use std::collections::HashMap;
 use std::convert::From;
 use std::env;
 use std::error::Error;
 use std::ffi::{OsStr, OsString};
 use std::fmt;
-use std::io::{Error as IoError, ErrorKind, Result as IoResult};
+use std::io;
+use std::io::ErrorKind;
 use std::path::{Path, PathBuf};
+use std::process;
 use std::process::{Child, Command, Stdio};
-use std::process;
+use std::thread;
+use std::time;
 
 pub trait Runner {
     type Process;
 
     fn arg<'a, S>(&'a mut self, arg: S) -> &'a mut Self
     where
         S: AsRef<OsStr>;
 
@@ -42,24 +44,50 @@ pub trait Runner {
     fn stderr<'a, T>(&'a mut self, stderr: T) -> &'a mut Self
     where
         T: Into<Stdio>;
 
     fn start(self) -> Result<Self::Process, RunnerError>;
 }
 
 pub trait RunnerProcess {
-    fn status(&mut self) -> IoResult<Option<process::ExitStatus>>;
-    fn stop(&mut self) -> IoResult<process::ExitStatus>;
-    fn is_running(&mut self) -> bool;
+    /// Attempts to collect the exit status of the process if it has already exited.
+    ///
+    /// This function will not block the calling thread and will only advisorily check to see if
+    /// the child process has exited or not.  If the process has exited then on Unix the process ID
+    /// is reaped.  This function is guaranteed to repeatedly return a successful exit status so
+    /// long as the child has already exited.
+    ///
+    /// If the process has exited, then `Ok(Some(status))` is returned.  If the exit status is not
+    /// available at this time then `Ok(None)` is returned.  If an error occurs, then that error is
+    /// returned.
+    fn try_wait(&mut self) -> io::Result<Option<process::ExitStatus>>;
+
+    /// Waits for the process to exit completely, killing it if it does not stop within `timeout`,
+    /// and returns the status that it exited with.
+    ///
+    /// Firefox' integrated background monitor observes long running threads during shutdown and
+    /// kills these after 63 seconds.  If the process fails to exit within the duration of
+    /// `timeout`, it is forcefully killed.
+    ///
+    /// This function will continue to have the same return value after it has been called at least
+    /// once.
+    fn wait(&mut self, timeout: time::Duration) -> io::Result<process::ExitStatus>;
+
+    /// Determine if the process is still running.
+    fn running(&mut self) -> bool;
+
+    /// Forces the process to exit and returns the exit status.  This is
+    /// equivalent to sending a SIGKILL on Unix platforms.
+    fn kill(&mut self) -> io::Result<process::ExitStatus>;
 }
 
 #[derive(Debug)]
 pub enum RunnerError {
-    Io(IoError),
+    Io(io::Error),
     PrefReader(PrefReaderError),
 }
 
 impl fmt::Display for RunnerError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         self.description().fmt(f)
     }
 }
@@ -80,44 +108,55 @@ impl Error for RunnerError {
     fn cause(&self) -> Option<&Error> {
         Some(match *self {
             RunnerError::Io(ref err) => err as &Error,
             RunnerError::PrefReader(ref err) => err as &Error,
         })
     }
 }
 
-impl From<IoError> for RunnerError {
-    fn from(value: IoError) -> RunnerError {
+impl From<io::Error> for RunnerError {
+    fn from(value: io::Error) -> RunnerError {
         RunnerError::Io(value)
     }
 }
 
 impl From<PrefReaderError> for RunnerError {
     fn from(value: PrefReaderError) -> RunnerError {
         RunnerError::PrefReader(value)
     }
 }
 
 #[derive(Debug)]
 pub struct FirefoxProcess {
     process: Child,
-    profile: Profile
+    profile: Profile,
 }
 
 impl RunnerProcess for FirefoxProcess {
-    fn status(&mut self) -> IoResult<Option<process::ExitStatus>> {
+    fn try_wait(&mut self) -> io::Result<Option<process::ExitStatus>> {
         self.process.try_wait()
     }
 
-    fn is_running(&mut self) -> bool {
-        self.status().unwrap().is_none()
+    fn wait(&mut self, timeout: time::Duration) -> io::Result<process::ExitStatus> {
+        let now = time::Instant::now();
+        while self.running() {
+            if now.elapsed() >= timeout {
+                break;
+            }
+            thread::sleep(time::Duration::from_millis(100));
+        }
+        self.kill()
     }
 
-    fn stop(&mut self) -> IoResult<process::ExitStatus> {
+    fn running(&mut self) -> bool {
+        self.try_wait().unwrap().is_none()
+    }
+
+    fn kill(&mut self) -> io::Result<process::ExitStatus> {
         self.process.kill()?;
         self.process.wait()
     }
 }
 
 #[derive(Debug)]
 pub struct FirefoxRunner {
     binary: PathBuf,