Remove Property constructor delegate

The delegate used for the Property constructor was
most frequently set to 'this' which in many cases
had not been completely initialized at the point
it was passed.  By not passing a delegate into the
Property constructor it will eliminate the
possibility that 'this' was passed before it was
initialized.
Make changes to Property uses to fit this new
mechanism.

BUG=b:253585974
TEST=USE="coverage" FEATURES="test noclean" emerge-brya chromeos-base/gestures

Change-Id: I1751163a3b945a6517e614ee720e7e1a3c49174e
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/gestures/+/4295788
Commit-Queue: Harry Cutts <hcutts@chromium.org>
Auto-Submit: Denis Brockus <dbrockus@chromium.org>
Code-Coverage: Harry Cutts <hcutts@chromium.org>
Tested-by: Denis Brockus <dbrockus@chromium.org>
Reviewed-by: Harry Cutts <hcutts@chromium.org>

7 files changed, 74 insertions, 80 deletions

diff --git a/include/prop_registry.h b/include/prop_registry.h
index 836b98b..4545a33 100644
--- a/include/prop_registry.h
+++ b/include/prop_registry.h
@@ -46,9 +46,8 @@ class PropertyDelegate;
 
 class Property {
  public:
-  Property(PropRegistry* parent, const char* name,
-           PropertyDelegate* delegate)
-      : gprop_(NULL), parent_(parent), delegate_(delegate), name_(name) {}
+  Property(PropRegistry* parent, const char* name)
+      : parent_(parent), name_(name) {}
 
   virtual ~Property() {
     if (parent_)
@@ -59,18 +58,9 @@ class Property {
   virtual void CreatePropImpl() = 0;
   void DestroyProp();
 
-  // b/253585974
-  // delegate used to get passed as a constructor parameter but that
-  // led to a chance that the delegate was 'this' and setting the
-  // delegate to 'this' in the initializer list allowed the property
-  // creation to use 'this' before it was initialized.  This could
-  // lead to unexpected behavior and if you were lucky to a crash.
-  //
-  // Now the delegate is always NULL on initilization of the property
-  // instance and after the delegate and property are completely
-  // created the user should set the delegate in the constructor
-  // body. This will allow access to this in a safe manner.
-  void SetDelegate(PropertyDelegate* delegate);
+  void SetDelegate(PropertyDelegate* delegate) {
+    delegate_ = delegate;
+  }
 
   const char* name() { return name_; }
   // Returns a newly allocated Value object
@@ -91,9 +81,9 @@ class Property {
   virtual void HandleGesturesPropWritten() = 0;
 
  protected:
-  GesturesProp* gprop_;
+  GesturesProp* gprop_ = NULL;
   PropRegistry* parent_;
-  PropertyDelegate* delegate_;
+  PropertyDelegate* delegate_ = NULL;
 
  private:
   const char* name_;
@@ -101,9 +91,8 @@ class Property {
 
 class BoolProperty : public Property {
  public:
-  BoolProperty(PropRegistry* reg, const char* name, GesturesPropBool val,
-               PropertyDelegate* delegate = NULL)
-      : Property(reg, name, delegate), val_(val) {
+  BoolProperty(PropRegistry* reg, const char* name, GesturesPropBool val)
+      : Property(reg, name), val_(val) {
     if (parent_)
       parent_->Register(this);
   }
@@ -118,8 +107,8 @@ class BoolProperty : public Property {
 class BoolArrayProperty : public Property {
  public:
   BoolArrayProperty(PropRegistry* reg, const char* name, GesturesPropBool* vals,
-                    size_t count, PropertyDelegate* delegate = NULL)
-      : Property(reg, name, delegate), vals_(vals), count_(count) {
+                    size_t count)
+      : Property(reg, name), vals_(vals), count_(count) {
     if (parent_)
       parent_->Register(this);
   }
@@ -134,9 +123,8 @@ class BoolArrayProperty : public Property {
 
 class DoubleProperty : public Property {
  public:
-  DoubleProperty(PropRegistry* reg, const char* name, double val,
-                 PropertyDelegate* delegate = NULL)
-      : Property(reg, name, delegate), val_(val) {
+  DoubleProperty(PropRegistry* reg, const char* name, double val)
+      : Property(reg, name), val_(val) {
     if (parent_)
       parent_->Register(this);
   }
@@ -151,8 +139,8 @@ class DoubleProperty : public Property {
 class DoubleArrayProperty : public Property {
  public:
   DoubleArrayProperty(PropRegistry* reg, const char* name, double* vals,
-                      size_t count, PropertyDelegate* delegate = NULL)
-      : Property(reg, name, delegate), vals_(vals), count_(count) {
+                      size_t count)
+      : Property(reg, name), vals_(vals), count_(count) {
     if (parent_)
       parent_->Register(this);
   }
@@ -167,9 +155,8 @@ class DoubleArrayProperty : public Property {
 
 class IntProperty : public Property {
  public:
-  IntProperty(PropRegistry* reg, const char* name, int val,
-              PropertyDelegate* delegate = NULL)
-      : Property(reg, name, delegate), val_(val) {
+  IntProperty(PropRegistry* reg, const char* name, int val)
+      : Property(reg, name), val_(val) {
     if (parent_)
       parent_->Register(this);
   }
@@ -183,9 +170,9 @@ class IntProperty : public Property {
 
 class IntArrayProperty : public Property {
  public:
-  IntArrayProperty(PropRegistry* reg, const char* name, int* vals, size_t count,
-                   PropertyDelegate* delegate = NULL)
-      : Property(reg, name, delegate), vals_(vals), count_(count) {
+  IntArrayProperty(PropRegistry* reg, const char* name, int* vals,
+                   size_t count)
+      : Property(reg, name), vals_(vals), count_(count) {
     if (parent_)
       parent_->Register(this);
   }
@@ -200,9 +187,8 @@ class IntArrayProperty : public Property {
 
 class StringProperty : public Property {
  public:
-  StringProperty(PropRegistry* reg, const char* name, const char* val,
-                 PropertyDelegate* delegate = NULL)
-      : Property(reg, name, delegate), val_(val) {
+  StringProperty(PropRegistry* reg, const char* name, const char* val)
+      : Property(reg, name), val_(val) {
     if (parent_)
       parent_->Register(this);
   }
diff --git a/src/iir_filter_interpreter.cc b/src/iir_filter_interpreter.cc
index 0558ef6..042e0f5 100644
--- a/src/iir_filter_interpreter.cc
+++ b/src/iir_filter_interpreter.cc
@@ -41,6 +41,7 @@ IirFilterInterpreter::IirFilterInterpreter(PropRegistry* prop_reg,
       a2_(prop_reg, "IIR a2", 0.412801598096189),
       iir_dist_thresh_(prop_reg, "IIR Distance Threshold", 10),
       adjust_iir_on_warp_(prop_reg, "Adjust IIR History On Warp", false) {
+  InitName();
   b0_.SetDelegate(this);
   b1_.SetDelegate(this);
   b2_.SetDelegate(this);
@@ -48,7 +49,6 @@ IirFilterInterpreter::IirFilterInterpreter(PropRegistry* prop_reg,
   a1_.SetDelegate(this);
   a2_.SetDelegate(this);
   iir_dist_thresh_.SetDelegate(this);
-  InitName();
 }
 
 void IirFilterInterpreter::SyncInterpretImpl(HardwareState* hwstate,
diff --git a/src/immediate_interpreter.cc b/src/immediate_interpreter.cc
index 70369eb..7af423c 100644
--- a/src/immediate_interpreter.cc
+++ b/src/immediate_interpreter.cc
@@ -1095,7 +1095,7 @@ ImmediateInterpreter::ImmediateInterpreter(PropRegistry* prop_reg,
       keyboard_touched_timeval_high_(prop_reg, "Keyboard Touched Timeval High",
                                      0),
       keyboard_touched_timeval_low_(prop_reg, "Keyboard Touched Timeval Low",
-                                    0, this),
+                                    0),
       keyboard_palm_prevent_timeout_(prop_reg, "Keyboard Palm Prevent Timeout",
                                      0.5),
       motion_tap_prevent_timeout_(prop_reg, "Motion Tap Prevent Timeout",
@@ -1136,6 +1136,7 @@ ImmediateInterpreter::ImmediateInterpreter(PropRegistry* prop_reg,
       quick_acceleration_factor_(prop_reg, "Quick Acceleration Factor", 0.0) {
   InitName();
   requires_metrics_ = true;
+  keyboard_touched_timeval_low_.SetDelegate(this);
 }
 
 void ImmediateInterpreter::SyncInterpretImpl(HardwareState* hwstate,
diff --git a/src/logging_filter_interpreter.cc b/src/logging_filter_interpreter.cc
index d504607..2c43238 100644
--- a/src/logging_filter_interpreter.cc
+++ b/src/logging_filter_interpreter.cc
@@ -17,15 +17,19 @@ LoggingFilterInterpreter::LoggingFilterInterpreter(PropRegistry* prop_reg,
                                                    Interpreter* next,
                                                    Tracer* tracer)
     : FilterInterpreter(prop_reg, next, tracer, true),
-      event_logging_enable_(prop_reg, "Event Logging Enable", false, this),
-      logging_notify_(prop_reg, "Logging Notify", 0, this),
-      logging_reset_(prop_reg, "Logging Reset", 0, this),
+      event_logging_enable_(prop_reg, "Event Logging Enable", false),
+      logging_notify_(prop_reg, "Logging Notify", 0),
+      logging_reset_(prop_reg, "Logging Reset", 0),
       log_location_(prop_reg, "Log Path",
                     "/var/log/xorg/touchpad_activity_log.txt"),
       integrated_touchpad_(prop_reg, "Integrated Touchpad", false) {
   InitName();
   if (prop_reg && log_.get())
     prop_reg->set_activity_log(log_.get());
+  event_logging_enable_.SetDelegate(this);
+  BoolWasWritten(&event_logging_enable_);
+  logging_notify_.SetDelegate(this);
+  logging_reset_.SetDelegate(this);
 }
 
 void LoggingFilterInterpreter::IntWasWritten(IntProperty* prop) {
diff --git a/src/mouse_interpreter.cc b/src/mouse_interpreter.cc
index b26de8f..f3055b3 100644
--- a/src/mouse_interpreter.cc
+++ b/src/mouse_interpreter.cc
@@ -29,8 +29,7 @@ MouseInterpreter::MouseInterpreter(PropRegistry* prop_reg, Tracer* tracer)
           scroll_accel_curve_, sizeof(scroll_accel_curve_) / sizeof(double)),
       scroll_max_allowed_input_speed_(prop_reg,
                                       "Mouse Scroll Max Input Speed",
-                                      177.0,
-                                      this),
+                                      177.0),
       force_scroll_wheel_emulation_(prop_reg,
                                      "Force Scroll Wheel Emulation",
                                      false),
@@ -53,6 +52,7 @@ MouseInterpreter::MouseInterpreter(PropRegistry* prop_reg, Tracer* tracer)
   scroll_accel_curve_[2] = 2.5737e-02;
   scroll_accel_curve_[3] = 8.0428e-05;
   scroll_accel_curve_[4] = -9.1149e-07;
+  scroll_max_allowed_input_speed_.SetDelegate(this);
 }
 
 void MouseInterpreter::SyncInterpretImpl(HardwareState* hwstate,
diff --git a/src/prop_registry.cc b/src/prop_registry.cc
index 58e3a1a..2d9d454 100644
--- a/src/prop_registry.cc
+++ b/src/prop_registry.cc
@@ -70,18 +70,6 @@ void Property::DestroyProp() {
   gprop_ = NULL;
 }
 
-void Property::SetDelegate(PropertyDelegate* delegate) {
-  bool delegate_was_late_set = !delegate_ && delegate;
-  delegate_ = delegate;
-  // In the normal path of creation of a property with a parent, the
-  // delegate is set late and the CreatePropImpl will not call the
-  // WasWritten method.  So catch the transition from NULL to not
-  // NULL in this condition and make sure to call the initial
-  // WasWritten to mimick the original code.
-  if (delegate_was_late_set && parent_ && parent_->PropProvider())
-    HandleGesturesPropWritten();
-}
-
 void BoolProperty::CreatePropImpl() {
   GesturesPropBool orig_val = val_;
   gprop_ = parent_->PropProvider()->create_bool_fn(
diff --git a/src/prop_registry_unittest.cc b/src/prop_registry_unittest.cc
index 23ba9d8..503a552 100644
--- a/src/prop_registry_unittest.cc
+++ b/src/prop_registry_unittest.cc
@@ -48,7 +48,8 @@ TEST(PropRegistryTest, SimpleTest) {
   PropRegistryTestDelegate delegate;
 
   int expected_call_cnt = 0;
-  BoolProperty bp1(&reg, "hi", false, &delegate);
+  BoolProperty bp1(&reg, "hi", false);
+  bp1.SetDelegate(&delegate);
   EXPECT_TRUE(strstr(ValueForProperty(bp1).c_str(), "false"));
   bp1.HandleGesturesPropWritten();
   EXPECT_EQ(++expected_call_cnt, delegate.call_cnt_);
@@ -58,7 +59,8 @@ TEST(PropRegistryTest, SimpleTest) {
   bp2.HandleGesturesPropWritten();
   EXPECT_EQ(expected_call_cnt, delegate.call_cnt_);
 
-  DoubleProperty dp1(&reg, "hi", 2721.0, &delegate);
+  DoubleProperty dp1(&reg, "hi", 2721.0);
+  dp1.SetDelegate(&delegate);
   EXPECT_TRUE(strstr(ValueForProperty(dp1).c_str(), "2721"));
   dp1.HandleGesturesPropWritten();
   EXPECT_EQ(++expected_call_cnt, delegate.call_cnt_);
@@ -68,7 +70,8 @@ TEST(PropRegistryTest, SimpleTest) {
   dp2.HandleGesturesPropWritten();
   EXPECT_EQ(expected_call_cnt, delegate.call_cnt_);
 
-  IntProperty ip1(&reg, "hi", 567, &delegate);
+  IntProperty ip1(&reg, "hi", 567);
+  ip1.SetDelegate(&delegate);
   EXPECT_TRUE(strstr(ValueForProperty(ip1).c_str(), "567"));
   ip1.HandleGesturesPropWritten();
   EXPECT_EQ(++expected_call_cnt, delegate.call_cnt_);
@@ -78,7 +81,8 @@ TEST(PropRegistryTest, SimpleTest) {
   ip2.HandleGesturesPropWritten();
   EXPECT_EQ(expected_call_cnt, delegate.call_cnt_);
 
-  StringProperty stp1(&reg, "hi", "foo", &delegate);
+  StringProperty stp1(&reg, "hi", "foo");
+  stp1.SetDelegate(&delegate);
   EXPECT_TRUE(strstr(ValueForProperty(stp1).c_str(), "foo"));
   stp1.HandleGesturesPropWritten();
   EXPECT_EQ(++expected_call_cnt, delegate.call_cnt_);
@@ -194,29 +198,31 @@ TEST(PropRegistryTest, SetAtCreateShouldNotifyTest) {
 
   PropRegistry reg;
   PropRegistryTestDelegate delegate;
-  BoolProperty my_bool(&reg, "MyBool", 0, &delegate);
-  DoubleProperty my_double(&reg, "MyDouble", 0.0, &delegate);
-  IntProperty my_int(&reg, "MyInt", 0, &delegate);
-  IntProperty my_int_no_change(&reg, "MyIntNoChange", 1, &delegate);
-  StringProperty my_string(&reg, "MyString", "mine", &delegate);
+  BoolProperty my_bool(&reg, "MyBool", 0);
+  my_bool.SetDelegate(&delegate);
+  DoubleProperty my_double(&reg, "MyDouble", 0.0);
+  my_double.SetDelegate(&delegate);
+  IntProperty my_int(&reg, "MyInt", 0);
+  my_int.SetDelegate(&delegate);
+  IntProperty my_int_no_change(&reg, "MyIntNoChange", 1);
+  my_int_no_change.SetDelegate(&delegate);
+  StringProperty my_string(&reg, "MyString", "mine");
+  my_string.SetDelegate(&delegate);
 
   EXPECT_EQ(0, delegate.call_cnt_);
   reg.SetPropProvider(&mock_gestures_props_provider, NULL);
   EXPECT_EQ(4, delegate.call_cnt_);
-
-  BoolProperty my_bool2(&reg, "MyBool2", 1);
-  EXPECT_EQ(4, delegate.call_cnt_);
-  my_bool2.SetDelegate(&delegate);
-  EXPECT_EQ(5, delegate.call_cnt_);
 }
 
 TEST(PropRegistryTest, DoublePromoteIntTest) {
   PropRegistry reg;
   PropRegistryTestDelegate delegate;
 
-  DoubleProperty my_double(&reg, "MyDouble", 1234.5, &delegate);
+  DoubleProperty my_double(&reg, "MyDouble", 1234.5);
+  my_double.SetDelegate(&delegate);
   EXPECT_TRUE(strstr(ValueForProperty(my_double).c_str(), "1234.5"));
-  IntProperty my_int(&reg, "MyInt", 321, &delegate);
+  IntProperty my_int(&reg, "MyInt", 321);
+  my_int.SetDelegate(&delegate);
   Json::Value my_int_val = my_int.NewValue();
   EXPECT_TRUE(my_double.SetValue(my_int_val));
   EXPECT_TRUE(strstr(ValueForProperty(my_double).c_str(), "321"));
@@ -229,15 +235,18 @@ TEST(PropRegistryTest, BoolArrayTest) {
 
   GesturesPropBool vals[] = { false, true };
   BoolArrayProperty my_bool_array_w_delegate(
-    &reg, "MyBoolArray", vals, 2, &delegate);
+    &reg, "MyBoolArray", vals, 2);
+  my_bool_array_w_delegate.SetDelegate(&delegate);
   EXPECT_EQ(0, delegate.call_cnt_);
   my_bool_array_w_delegate.HandleGesturesPropWritten();
   EXPECT_EQ(1, delegate.call_cnt_);
   delegate.BoolArrayWasWritten(&my_bool_array_w_delegate);
   EXPECT_EQ(2, delegate.call_cnt_);
 
-  IntProperty ip1(&reg, "hi", 567, &delegate);
-  StringProperty stp1(&reg, "hi", "foo", &delegate);
+  IntProperty ip1(&reg, "hi", 567);
+  ip1.SetDelegate(&delegate);
+  StringProperty stp1(&reg, "hi", "foo");
+  stp1.SetDelegate(&delegate);
   Json::Value my_bool_array_val = my_bool_array_w_delegate.NewValue();
   Json::Value my_int_val = ip1.NewValue();
   Json::Value my_str_val = stp1.NewValue();
@@ -261,15 +270,18 @@ TEST(PropRegistryTest, DoubleArrayTest) {
 
   double vals[] = { 0.0, 1.0 };
   DoubleArrayProperty my_double_array_w_delegate(
-    &reg, "MyDoubleArray", vals, 2, &delegate);
+    &reg, "MyDoubleArray", vals, 2);
+  my_double_array_w_delegate.SetDelegate(&delegate);
   EXPECT_EQ(0, delegate.call_cnt_);
   my_double_array_w_delegate.HandleGesturesPropWritten();
   EXPECT_EQ(1, delegate.call_cnt_);
   delegate.DoubleArrayWasWritten(&my_double_array_w_delegate);
   EXPECT_EQ(2, delegate.call_cnt_);
 
-  IntProperty ip1(&reg, "hi", 567, &delegate);
-  StringProperty stp1(&reg, "hi", "foo", &delegate);
+  IntProperty ip1(&reg, "hi", 567);
+  ip1.SetDelegate(&delegate);
+  StringProperty stp1(&reg, "hi", "foo");
+  stp1.SetDelegate(&delegate);
   Json::Value my_double_array_val = my_double_array_w_delegate.NewValue();
   Json::Value my_int_val = ip1.NewValue();
   Json::Value my_str_val = stp1.NewValue();
@@ -293,15 +305,18 @@ TEST(PropRegistryTest, IntArrayTest) {
 
   int vals[] = { 0, 1 };
   IntArrayProperty my_int_array_w_delegate(
-    &reg, "MyIntArray", vals, 2, &delegate);
+    &reg, "MyIntArray", vals, 2);
+  my_int_array_w_delegate.SetDelegate(&delegate);
   EXPECT_EQ(0, delegate.call_cnt_);
   my_int_array_w_delegate.HandleGesturesPropWritten();
   EXPECT_EQ(1, delegate.call_cnt_);
   delegate.IntArrayWasWritten(&my_int_array_w_delegate);
   EXPECT_EQ(2, delegate.call_cnt_);
 
-  IntProperty ip1(&reg, "hi", 567, &delegate);
-  StringProperty stp1(&reg, "hi", "foo", &delegate);
+  IntProperty ip1(&reg, "hi", 567);
+  ip1.SetDelegate(&delegate);
+  StringProperty stp1(&reg, "hi", "foo");
+  stp1.SetDelegate(&delegate);
   Json::Value my_int_array_val = my_int_array_w_delegate.NewValue();
   Json::Value my_int_val = ip1.NewValue();
   Json::Value my_str_val = stp1.NewValue();