Refactor scan filter

3 files changed, 91 insertions, 126 deletions

diff --git a/components/net/bluetooth_thread.rs b/components/net/bluetooth_thread.rs
index 82fe4891ee9..e6068fa37a9 100644
--- a/components/net/bluetooth_thread.rs
+++ b/components/net/bluetooth_thread.rs
@@ -8,7 +8,7 @@ use device::bluetooth::BluetoothGATTCharacteristic;
 use device::bluetooth::BluetoothGATTDescriptor;
 use device::bluetooth::BluetoothGATTService;
 use ipc_channel::ipc::{self, IpcReceiver, IpcSender};
-use net_traits::bluetooth_scanfilter::{RequestDeviceoptions, matches_filters};
+use net_traits::bluetooth_scanfilter::{BluetoothScanfilter, BluetoothScanfilterSequence, RequestDeviceoptions};
 use net_traits::bluetooth_thread::{BluetoothMethodMsg, BluetoothObjectMsg};
 use std::borrow::ToOwned;
 use std::collections::HashMap;
@@ -55,6 +55,47 @@ impl BluetoothThreadFactory for IpcSender<BluetoothMethodMsg> {
     }
 }
 
+fn matches_filter(device: &BluetoothDevice, filter: &BluetoothScanfilter) -> bool {
+    if filter.is_empty_or_invalid() {
+        return false;
+    }
+
+    if !filter.get_name().is_empty() {
+        if device.get_name().ok() != Some(filter.get_name().to_string()) {
+            return false;
+        }
+    }
+
+    if !filter.get_name_prefix().is_empty() {
+        if let Ok(device_name) = device.get_name() {
+            if !device_name.starts_with(filter.get_name_prefix()) {
+                return false;
+            }
+        } else {
+            return false;
+        }
+    }
+
+    if !filter.get_services().is_empty() {
+        if let Ok(device_uuids) = device.get_uuids() {
+            for service in filter.get_services() {
+                if device_uuids.iter().find(|x| x == &service).is_none() {
+                    return false;
+                }
+            }
+        }
+    }
+    return true;
+}
+
+fn matches_filters(device: &BluetoothDevice, filters: &BluetoothScanfilterSequence) -> bool {
+    if filters.has_empty_or_invalid_filter() {
+        return false;
+    }
+
+    return filters.iter().any(|f| matches_filter(device, f))
+}
+
 pub struct BluetoothManager {
     receiver: IpcReceiver<BluetoothMethodMsg>,
     adapter: Option<BluetoothAdapter>,
diff --git a/components/net_traits/bluetooth_scanfilter.rs b/components/net_traits/bluetooth_scanfilter.rs
index 58eb72f518e..b332c1a52b2 100644
--- a/components/net_traits/bluetooth_scanfilter.rs
+++ b/components/net_traits/bluetooth_scanfilter.rs
@@ -2,11 +2,11 @@
  * 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/. */
 
-use device::bluetooth::BluetoothDevice;
+use std::slice::Iter;
 
 // A device name can never be longer than 29 bytes. An adv packet is at most
 // 31 bytes long. The length and identifier of the length field take 2 bytes.
-// That least 29 bytes for the name.
+// That leaves 29 bytes for the name.
 const MAX_NAME_LENGTH: usize = 29;
 
 #[derive(Deserialize, Serialize)]
@@ -16,10 +16,6 @@ impl ServiceUUIDSequence {
     pub fn new(vec: Vec<String>) -> ServiceUUIDSequence {
         ServiceUUIDSequence(vec)
     }
-
-    fn is_empty(&self) -> bool {
-        self.0.is_empty()
-    }
 }
 
 #[derive(Deserialize, Serialize)]
@@ -30,14 +26,6 @@ pub struct BluetoothScanfilter {
 }
 
 impl BluetoothScanfilter {
-    fn is_empty_or_invalid_filter(&self) -> bool {
-        self.name.is_empty() &&
-        self.name_prefix.is_empty() &&
-        self.services.is_empty() &&
-        self.name.len() > MAX_NAME_LENGTH &&
-        self.name_prefix.len() > MAX_NAME_LENGTH
-    }
-
     pub fn new(name: String, name_prefix: String, services: Vec<String>) -> BluetoothScanfilter {
         BluetoothScanfilter {
             name: name,
@@ -45,6 +33,24 @@ impl BluetoothScanfilter {
             services: ServiceUUIDSequence::new(services),
         }
     }
+
+    pub fn get_name(&self) -> &str {
+        &self.name
+    }
+
+    pub fn get_name_prefix(&self) -> &str {
+        &self.name_prefix
+    }
+
+    pub fn get_services(&self) -> &[String] {
+        &self.services.0
+    }
+
+    pub fn is_empty_or_invalid(&self) -> bool {
+        (self.name.is_empty() && self.name_prefix.is_empty() && self.get_services().is_empty()) ||
+        self.name.len() > MAX_NAME_LENGTH ||
+        self.name_prefix.len() > MAX_NAME_LENGTH
+    }
 }
 
 #[derive(Deserialize, Serialize)]
@@ -54,12 +60,14 @@ impl BluetoothScanfilterSequence {
     pub fn new(vec: Vec<BluetoothScanfilter>) -> BluetoothScanfilterSequence {
         BluetoothScanfilterSequence(vec)
     }
-}
 
-impl BluetoothScanfilterSequence {
-    fn has_empty_or_invalid_filter(&self) -> bool {
-        self.0.is_empty() &&
-        self.0.iter().all(|x| !(x.is_empty_or_invalid_filter()))
+    pub fn has_empty_or_invalid_filter(&self) -> bool {
+        self.0.is_empty() ||
+        self.0.iter().any(BluetoothScanfilter::is_empty_or_invalid)
+    }
+
+    pub fn iter(&self) -> Iter<BluetoothScanfilter> {
+        self.0.iter()
     }
 }
 
@@ -83,53 +91,3 @@ impl RequestDeviceoptions {
         &self.filters
     }
 }
-
-pub fn matches_filter(device: &BluetoothDevice, filter: &BluetoothScanfilter) -> bool {
-    if filter.is_empty_or_invalid_filter() {
-        return false;
-    }
-
-     if !filter.name.is_empty() {
-        if let Ok(device_name) = device.get_name() {
-            if !device_name.eq(&filter.name) {
-                return false;
-            }
-        } else {
-            return false;
-        }
-    }
-
-    if !filter.name_prefix.is_empty() {
-        if let Ok(device_name) = device.get_name() {
-            if !device_name.starts_with(&*filter.name_prefix) {
-                return false;
-            }
-        } else {
-            return false;
-        }
-    }
-
-    if !filter.services.is_empty() {
-        if let Ok(stringvec) = device.get_uuids() {
-            for service in &filter.services.0 {
-                if !stringvec.iter().any(|x| x == service) {
-                    return false;
-                }
-            }
-        }
-    }
-    return true;
-}
-
-pub fn matches_filters(device: &BluetoothDevice, filters: &BluetoothScanfilterSequence) -> bool {
-    if filters.has_empty_or_invalid_filter() {
-        return false;
-    }
-
-    for filter in &filters.0 {
-        if matches_filter(device, filter) {
-            return true;
-        }
-    }
-    return false;
-}
diff --git a/components/script/dom/bluetooth.rs b/components/script/dom/bluetooth.rs
index d56528d845a..46602b4bbf3 100644
--- a/components/script/dom/bluetooth.rs
+++ b/components/script/dom/bluetooth.rs
@@ -21,14 +21,15 @@ use net_traits::bluetooth_scanfilter::{RequestDeviceoptions, ServiceUUIDSequence
 use net_traits::bluetooth_thread::{BluetoothMethodMsg, BluetoothObjectMsg};
 use util::str::DOMString;
 
-// A device name can never be longer than 29 bytes. An adv packet is at most
-// 31 bytes long. The length and identifier of the length field take 2 bytes.
 const FILTER_EMPTY_ERROR: &'static str = "'filters' member must be non - empty to find any devices.";
 const FILTER_ERROR: &'static str = "A filter must restrict the devices in some way.";
 const FILTER_NAME_TOO_LONG_ERROR: &'static str = "A 'name' or 'namePrefix' can't be longer then 29 bytes.";
 // 248 is the maximum number of UTF-8 code units in a Bluetooth Device Name.
 const MAX_DEVICE_NAME_LENGTH: usize = 248;
-// That least 29 bytes for the name.
+// A device name can never be longer than 29 bytes.
+// An advertising packet is at most 31 bytes long.
+// The length and identifier of the length field take 2 bytes.
+// That leaves 29 bytes for the name.
 const MAX_FILTER_NAME_LENGTH: usize = 29;
 const NAME_PREFIX_ERROR: &'static str = "'namePrefix', if present, must be non - empty.";
 const NAME_TOO_LONG_ERROR: &'static str = "A device name can't be longer than 248 bytes.";
@@ -60,45 +61,23 @@ impl Bluetooth {
     }
 }
 
-impl Clone for BluetoothScanFilter {
-    fn clone(&self) -> BluetoothScanFilter {
-        BluetoothScanFilter {
-            name: self.name.clone(),
-            namePrefix: self.namePrefix.clone(),
-            services: self.services.clone(),
-        }
-    }
-}
-
-impl Clone for RequestDeviceOptions {
-    fn clone(&self) -> RequestDeviceOptions {
-        RequestDeviceOptions {
-            filters: self.filters.clone(),
-            optionalServices: self.optionalServices.clone(),
-        }
-    }
-}
-
 fn canonicalize_filter(filter: &BluetoothScanFilter, global: GlobalRef) -> Fallible<BluetoothScanfilter> {
-    if !(filter.services.is_some() || filter.name.is_some() || filter.namePrefix.is_some()) {
+    if filter.services.is_none() && filter.name.is_none() && filter.namePrefix.is_none() {
         return Err(Type(FILTER_ERROR.to_owned()));
     }
 
-    let mut services_vec: Vec<String> = vec!();
-    if let Some(services) = filter.services.clone() {
+    let mut services_vec = vec!();
+    if let Some(ref services) = filter.services {
         if services.is_empty() {
             return Err(Type(SERVICE_ERROR.to_owned()));
         }
         for service in services {
-            match BluetoothUUID::GetService(global, service) {
-                Ok(valid) => services_vec.push(valid.to_string()),
-                Err(err) => return Err(err),
-            }
+            services_vec.push(try!(BluetoothUUID::GetService(global, service.clone())).to_string());
         }
     }
 
     let mut name = String::new();
-    if let Some(filter_name) = filter.name.clone() {
+    if let Some(ref filter_name) = filter.name {
         //NOTE: DOMString::len() gives back the size in bytes
         if filter_name.len() > MAX_DEVICE_NAME_LENGTH {
             return Err(Type(NAME_TOO_LONG_ERROR.to_owned()));
@@ -110,8 +89,8 @@ fn canonicalize_filter(filter: &BluetoothScanFilter, global: GlobalRef) -> Falli
     }
 
     let mut name_prefix = String::new();
-    if let Some(filter_name_prefix) = filter.namePrefix.clone() {
-        if filter_name_prefix.len() == 0 {
+    if let Some(ref filter_name_prefix) = filter.namePrefix {
+        if filter_name_prefix.is_empty() {
             return Err(Type(NAME_PREFIX_ERROR.to_owned()));
         }
         if filter_name_prefix.len() > MAX_DEVICE_NAME_LENGTH {
@@ -135,19 +114,13 @@ fn convert_request_device_options(options: &RequestDeviceOptions,
 
     let mut filters = vec!();
     for filter in &options.filters {
-        match canonicalize_filter(&filter, global) {
-            Ok(canonicalized_filter) => filters.push(canonicalized_filter),
-            Err(err) => return Err(err),
-        }
+        filters.push(try!(canonicalize_filter(&filter, global)));
     }
 
     let mut optional_services = vec!();
-    if let Some(opt_services) = options.optionalServices.clone() {
+    if let Some(ref opt_services) = options.optionalServices {
         for opt_service in opt_services {
-            match BluetoothUUID::GetService(global, opt_service) {
-                Ok(valid_service) => optional_services.push(valid_service.to_string()),
-                Err(err) => return Err(err),
-            }
+            optional_services.push(try!(BluetoothUUID::GetService(global, opt_service.clone())).to_string());
         }
     }
 
@@ -160,8 +133,8 @@ impl BluetoothMethods for Bluetooth {
     // https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetooth-requestdevice
     fn RequestDevice(&self, option: &RequestDeviceOptions) -> Fallible<Root<BluetoothDevice>> {
         let (sender, receiver) = ipc::channel().unwrap();
-        match convert_request_device_options(option, self.global().r()) {
-            Ok(option) => {
+        match try!(convert_request_device_options(option, self.global().r())) {
+            option => {
                 self.get_bluetooth_thread().send(
                     BluetoothMethodMsg::RequestDevice(option, sender)).unwrap();
                 let device = receiver.recv().unwrap();
@@ -182,18 +155,12 @@ impl BluetoothMethods for Bluetooth {
                                                                      appearance,
                                                                      tx_power,
                                                                      rssi);
-                        let vendor_id_source = match vendor_id_source {
-                            Some(vid) => match vid.as_ref() {
-                                "bluetooth" => Some(VendorIDSource::Bluetooth),
-                                "usb" => Some(VendorIDSource::Usb),
-                                _ => Some(VendorIDSource::Unknown),
-                            },
-                            None => None,
-                        };
-                        let name = match name {
-                            Some(n) => Some(DOMString::from(n)),
-                            None => None,
-                        };
+                        let vendor_id_source = vendor_id_source.map(|vid| match vid.as_str() {
+                            "bluetooth" => VendorIDSource::Bluetooth,
+                            "usb" => VendorIDSource::Usb,
+                            _ => VendorIDSource::Unknown,
+                        });
+                        let name = name.map(DOMString::from);
                         Ok(BluetoothDevice::new(self.global().r(),
                                                 DOMString::from(id),
                                                 name,
@@ -212,7 +179,6 @@ impl BluetoothMethods for Bluetooth {
                     _ => unreachable!()
                 }
             },
-            Err(err) => Err(err),
         }
     }
 }