Add |notify_client_on_reading_change| flag to sensor configuration

device/generic_sensor/public/interfaces/sensor.mojom

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 module device.mojom;
 
 // Types of supported sensors
 // When adding new sensor type, update the documentation of sensor data values
 // in SensorReading struct at sensor_reading.h file.
 enum SensorType {
@@ skipping 22 matching lines @@
   CONTINUOUS
 };
 
 struct SensorConfiguration {
   // Maximum allowed frequency is 60 Hz.
   const double kMaxAllowedFrequency = 60.0;
 
   // Requested frequency in Hz.
   double frequency;
   // TODO(shalamov): Add map<string, union> for sensor specific configuration.
+
+  // For sensors with ON_CHANGE reporting mode:
+  // If at least one configuration sets this flag to true,
+  // SensorClient::SensorReadingChanged() signal is not sent to the client;
+  // otherwise it is.
+  // For sensors with CONTINUOUS reporting mode:
+  // This flag has no effect since SensorClient::SensorReadingChanged() is not
+  // sent to the client.
+  bool suppress_on_change_events = false;

[Mikhail, 2017/06/12 11:01:10]

Shouldn't it be rather added to SensorInitParams (sensor_provider.mojom)?

Think the more elegant way is to modify PlatformSensor::Client API (pls look at
https://cs.chromium.org/chromium/src/device/generic_sensor/platform_sensor.cc...
), Client::IsNotificationSuspended() could be renamed to
'IsOnChangeSuppressed()' or smth like that.

[juncai, 2017/06/12 17:32:10]

If I understand it correctly, SensorInitParams is the return value of the mojo
IPC function call GetSensor:
https://cs.chromium.org/chromium/src/device/generic_sensor/public/interfaces/...
and the renderer side can't configure or make change of this parameter.

The purpose of |suppress_on_change_events| is to let renderer side be able to
control if receiving SensorReadingChanged() calls.

[Mikhail, 2017/06/13 12:54:56]

Oh I see. My only concern is that 'SensorConfiguration' is supposed to contain
something to be passed to the underlying platform sensor,
whereas the newly added "suppress_on_change_events" field only affects the
behavior of the MOJO API implementation.

The possible alternate solution would be the following API refactoring

interface SensorChangeListener {
  // Signals SensorChangeListener when reading has been changed.
  SensorReadingChanged();
};

interface Sensor {
  // The listener is set if the client wants to recieve 'onchange' notification
via IPC (and not set otherwise!).
  SetChangeListener(SensorChangeListener);
};

interface SensorClient {
  // Signals SensorClient when there is an error.
  RaiseError();
  // Does not contain SensorReadingChanged() any more.
};

I'm fine with landing this CL as it is for now and making the refactoring in
future (I could take this burden).

 };
 
 // Interface for controlling the Sensor.
 interface Sensor {
 
   // Requests sensor to provide its default configuration.
   GetDefaultConfiguration() => (SensorConfiguration configuration);
 
   // Requests sensor to start reading sensor data with specified
   // SensorConfiguration.
@@ skipping 29 matching lines @@
 // Interface that client of the Sensor interface must implement to observe
 // sensor reading changes and error conditions.
 interface SensorClient {
   // Signals SensorClient when there is an error.
   RaiseError();
 
   // Signals SensorClient when reading has been changed (only for sensors with
   // ReportingMode::ON_CHANGE).
   SensorReadingChanged();
 };