C++ readability review

components/timers/rtc_alarm.h

 // Copyright 2014 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.
 
 #ifndef COMPONENTS_TIMER_RTC_ALARM_H_
 #define COMPONENTS_TIMER_RTC_ALARM_H_
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 55 matching lines @@
   scoped_refptr<base::MessageLoopProxy> origin_message_loop_;
 
   // The parent timer that should be informed when the timer fires.  We may end
   // up outliving the parent so we need to ensure the reference is valid before
   // we try to call it.
   base::WeakPtr<AlarmTimer> parent_;
 
   // Manages watching file descriptors.
   scoped_ptr<base::MessageLoopForIO::FileDescriptorWatcher> fd_watcher_;
 
   // These two variables are used for coordinating between the thread that

[gromer, 2014/12/05 20:13:41]

Comments that say what something is "used for" tend to be unhelpfully vague,
especially for the opening sentence, because they don't tell me specifically how
to use them. Instead, try to talk about what the variable _is_, or what it
_means_. For example, "The ID of the most recent Reset() call handled by
origin_message_loop_" (I have no idea if that's correct, but it's the kind of
comment I'm looking for).

Relatedly, "event" is a very vague name. Especially in such subtle code, you
should choose variable names that give readers specific cues about the meaning
of the variable. For example, if I'm right that the events here are really
Reset() calls, you could call them origin_reset_event_id_ and io_reset_event_id.

Finally, the term "sequence number" is often used with this kind of scheme;
consider saying that instead of "event ID".

[Chirantan Ekbote, 2014/12/31 00:58:29]

Ack.  I'll try and clean up the explanation.

   // started the timer and the IO thread being used to watch the timer file
   // descriptor.  When Reset() is called, the original thread increments
   // |origin_event_id_| and binds its value to ResetImpl(), which gets posted to
   // the IO thread.  When the IO thread runs, it saves this value in
   // |io_event_id_|.  Later, when the timer fires, the IO thread binds the value
   // of |io_event_id_| to OnTimerFired() and posts it to the original thread.
   // When the original thread runs OnTimerFired(), it calls
   // parent_->OnTimerFired() only if |origin_event_id_| matches the event id
   // that was passed in to it.  This is used to get around a race condition
   // where the user resets the timer on the original thread, while the event is
   // being fired on the IO thread at the same time.
   int origin_event_id_;
   int io_event_id_;
 
   DISALLOW_COPY_AND_ASSIGN(RtcAlarm);
 };
 
+
 }  // namespace timers
 #endif  // COMPONENTS_TIMER_RTC_ALARM_H_