Merge changes I442b7919,Ie5b4ff75

* changes:
  Improve android MediaDrm property latency
  Delay OEMCrypto Termination

libwvdrmengine/cdm/core/include/crypto_session.h

@@ -44,11 +44,11 @@ class CryptoSessionFactory;
 class CryptoSession {
  public:
   using HdcpCapability = OEMCrypto_HDCP_Capability;
-  typedef enum {
+  enum UsageDurationStatus {
     kUsageDurationsInvalid = 0,
     kUsageDurationPlaybackNotBegun = 1,
     kUsageDurationsValid = 2,
-  } UsageDurationStatus;
+  };
 
   struct SupportedCertificateTypes {
     bool rsa_2048_bit;
@@ -64,6 +64,17 @@ class CryptoSession {
 
   virtual ~CryptoSession();
 
+  // This method will try to terminate OEMCrypto if |session_size_| is 0.
+  // A platform configured property |delay_oem_crypto_termination| will
+  // determine if termination occurs immediately or after a delay.
+  // If termination is delayed, a countdown mechanism is employed.
+  // Call |TryTerminate| periodically until it no longer returns true.
+  // To immediately terminate call |DisableDelayedTermination| before calling
+  // |TryTerminate|.
+  static bool TryTerminate();
+
+  static void DisableDelayedTermination();
+
   virtual CdmResponseType GetProvisioningToken(std::string* client_token);
   virtual CdmClientTokenType GetPreProvisionTokenType() {
     return pre_provision_token_type_;
@@ -282,7 +293,6 @@ class CryptoSession {
   }
 
   void Init();
-  void Terminate();
   CdmResponseType GetTokenFromKeybox(std::string* token);
   CdmResponseType GetTokenFromOemCert(std::string* token);
   static bool ExtractSystemIdFromOemCert(const std::string& oem_cert,
@@ -385,6 +395,7 @@ class CryptoSession {
 
   static bool initialized_;
   static int session_count_;
+  static int termination_counter_;
 
   metrics::CryptoMetrics* metrics_;
   metrics::TimerMetric life_span_;

libwvdrmengine/cdm/core/include/properties.h

@@ -61,6 +61,9 @@ class Properties {
   static void set_provisioning_messages_are_binary(bool flag) {
     provisioning_messages_are_binary_ = flag;
   }
+  static inline bool delay_oem_crypto_termination() {
+    return delay_oem_crypto_termination_;
+  }
   static bool GetCompanyName(std::string* company_name);
   static bool GetModelName(std::string* model_name);
   static bool GetArchitectureName(std::string* arch_name);
@@ -147,6 +150,7 @@ class Properties {
   static bool allow_service_certificate_requests_;
   static bool device_files_is_a_real_filesystem_;
   static bool allow_restore_of_offline_licenses_with_release_;
+  static bool delay_oem_crypto_termination_;
   static std::unique_ptr<CdmClientPropertySetMap> session_property_set_;
 
   CORE_DISALLOW_COPY_AND_ASSIGN(Properties);

libwvdrmengine/cdm/core/src/crypto_session.cpp

@@ -60,6 +60,7 @@ const size_t kOemCryptoApiVersionSupportsSwitchingCipherMode = 14;
 
 // Constants and utility objects relating to OEM Certificates
 const char* const kWidevineSystemIdExtensionOid = "1.3.6.1.4.1.11129.4.1.1";
+constexpr int kMaxTerminateCountDown = 5;
 
 const std::string kStringNotAvailable = "NA";
 
@@ -96,6 +97,7 @@ shared_mutex CryptoSession::static_field_mutex_;
 shared_mutex CryptoSession::oem_crypto_mutex_;
 bool CryptoSession::initialized_ = false;
 int CryptoSession::session_count_ = 0;
+int CryptoSession::termination_counter_ = 0;
 UsageTableHeader* CryptoSession::usage_table_header_l1_ = nullptr;
 UsageTableHeader* CryptoSession::usage_table_header_l3_ = nullptr;
 std::atomic<uint64_t> CryptoSession::request_id_index_source_(0);
@@ -189,7 +191,15 @@ CryptoSession::~CryptoSession() {
   if (open_) {
     Close();
   }
-  Terminate();
+  WithStaticFieldWriteLock("~CryptoSession", [&] {
+    if (session_count_ > 0) {
+      --session_count_;
+    } else {
+      LOGE("Invalid crypto session count: session_count_ = %d", session_count_);
+    }
+  });
+
+  TryTerminate();
   M_RECORD(metrics_, crypto_session_life_span_, life_span_.AsMs());
 }
 
@@ -244,6 +254,9 @@ void CryptoSession::Init() {
         LOGE("OEMCrypto_Initialize failed: status = %d", static_cast<int>(sts));
         return;
       }
+      termination_counter_ = Properties::delay_oem_crypto_termination()
+                                 ? kMaxTerminateCountDown
+                                 : 0;
       initialized_ = true;
       initialized = true;
     }
@@ -273,17 +286,20 @@ void CryptoSession::Init() {
   }
 }
 
-void CryptoSession::Terminate() {
+bool CryptoSession::TryTerminate() {
   LOGV("Terminating crypto session");
-  WithStaticFieldWriteLock("Terminate", [&] {
-    LOGV("Terminating crypto session: initialized_ = %s, session_count_ = %d",
-         initialized_ ? "true" : "false", session_count_);
-    if (session_count_ > 0) {
-      session_count_ -= 1;
-    } else {
-      LOGE("Invalid crypto session count: session_count_ = %d", session_count_);
+  WithStaticFieldWriteLock("TryTerminate", [&] {
+    LOGV(
+        "Terminating crypto session: initialized_ = %s, session_count_ = %d, "
+        "termination_counter_ = %d",
+        initialized_ ? "true" : "false", session_count_, termination_counter_);
+
+    if (termination_counter_ > 0) {
+      --termination_counter_;
     }
-    if (session_count_ > 0 || !initialized_) return;
+    if (session_count_ > 0 || termination_counter_ > 0 || !initialized_)
+      return false;
+
     OEMCryptoResult sts;
     WithOecWriteLock("Terminate", [&] { sts = OEMCrypto_Terminate(); });
     if (OEMCrypto_SUCCESS != sts) {
@@ -300,7 +316,15 @@ void CryptoSession::Terminate() {
     }
 
     initialized_ = false;
+    return true;
   });
+  return true;
+}
+
+void CryptoSession::DisableDelayedTermination() {
+  LOGV("Disable delayed termination");
+  WithStaticFieldWriteLock("DisableDelayedTermination",
+                           [&] { termination_counter_ = 0; });
 }
 
 CdmResponseType CryptoSession::GetTokenFromKeybox(std::string* token) {
@@ -687,6 +711,11 @@ CdmResponseType CryptoSession::Open(SecurityLevel requested_security_level) {
     return MapOEMCryptoResult(sts, OPEN_CRYPTO_SESSION_ERROR, "Open");
   }
 
+  WithStaticFieldWriteLock("Open() termination_counter", [&] {
+    termination_counter_ =
+        Properties::delay_oem_crypto_termination() ? kMaxTerminateCountDown : 0;
+  });
+
   oec_session_id_ = static_cast<CryptoSessionId>(sid);
   LOGV("Opened session: id = %u", oec_session_id_);
   open_ = true;

libwvdrmengine/cdm/core/src/properties.cpp

@@ -21,6 +21,7 @@ bool Properties::provisioning_messages_are_binary_;
 bool Properties::allow_service_certificate_requests_;
 bool Properties::device_files_is_a_real_filesystem_;
 bool Properties::allow_restore_of_offline_licenses_with_release_;
+bool Properties::delay_oem_crypto_termination_;
 std::unique_ptr<CdmClientPropertySetMap> Properties::session_property_set_;
 
 bool Properties::AddSessionPropertySet(const CdmSessionId& session_id,

libwvdrmengine/cdm/include/properties_configuration.h

@@ -39,6 +39,11 @@ const bool kDeviceFilesIsARealFileSystem = true;
 // will be treated as a release request. If false, a restoration will fail.
 const bool kAllowRestoreOfflineLicenseWithRelease = false;
 
+// This controls whether an OEMCrypto_Terminate call is issued immediately
+// after crypto session count declines to 0 or if a termination is
+// delayed by a countdown timer. If false, termination is immediate.
+const bool kPropertyDelayOemCryptoTermination = true;
+
 }  // namespace wvcdm
 
 #endif  // CDM_BASE_WV_PROPERTIES_CONFIGURATION_H_

libwvdrmengine/cdm/include/timer.h

@@ -43,7 +43,10 @@ class Timer {
   ~Timer();
 
   bool Start(TimerHandler* handler, uint32_t time_in_secs);
-  void Stop();
+  // Enable |wait_for_exit| only if the method is being invoked from a thread
+  // other than the timer thread. Deadlock might occur if enabled and called
+  // from the timer thread.
+  void Stop(bool wait_for_exit);
   bool IsRunning();
 
  private:

libwvdrmengine/cdm/include/wv_content_decryption_module.h

@@ -184,14 +184,17 @@ class WvContentDecryptionModule : public android::RefBase, public TimerHandler {
 
   uint32_t GenerateSessionSharingId();
 
-  // timer related methods to drive policy decisions
-  void EnablePolicyTimer();
-  void DisablePolicyTimer();
+  // Timer related methods to drive policy decisions
+  void EnableTimer();
+  void DisableTimer();
+  // Call this method only if invoked from a thread other than the timer thread.
+  // Otherwise this might result in deadlock.
+  void DisableTimerAndWaitForExit();
   void OnTimerEvent();
 
   static std::mutex session_sharing_id_generation_lock_;
-  std::mutex policy_timer_lock_;
-  Timer policy_timer_;
+  std::mutex timer_lock_;
+  Timer timer_;
 
   // instance variables
   // This manages the lifetime of the CDM instances.

libwvdrmengine/cdm/src/properties_android.cpp

@@ -52,6 +52,7 @@ void Properties::InitOnce() {
   device_files_is_a_real_filesystem_ = kDeviceFilesIsARealFileSystem;
   allow_restore_of_offline_licenses_with_release_ =
       kAllowRestoreOfflineLicenseWithRelease;
+  delay_oem_crypto_termination_ = kPropertyDelayOemCryptoTermination;
   session_property_set_.reset(new CdmClientPropertySetMap());
 }
 

libwvdrmengine/cdm/src/timer.cpp

@@ -28,12 +28,13 @@ class Timer::Impl : virtual public android::RefBase {
       return run("wvcdm::Timer::Impl") == android::NO_ERROR;
     }
 
-    void Stop() {
-      {
-        android::Mutex::Autolock autoLock(lock_);
-        stop_condition_.signal();
+    void Stop(bool wait_for_exit) {
+      stop_condition_.signal();
+      if (wait_for_exit) {
+        requestExitAndWait();
+      } else {
+        requestExit();
       }
-      requestExitAndWait();
     }
 
    private:
@@ -63,8 +64,8 @@ class Timer::Impl : virtual public android::RefBase {
     return impl_thread_->Start(handler, time_in_secs);
   }
 
-  void Stop() {
-    impl_thread_->Stop();
+  void Stop(bool wait_for_exit) {
+    impl_thread_->Stop(wait_for_exit);
     impl_thread_.clear();
   }
 
@@ -78,7 +79,7 @@ class Timer::Impl : virtual public android::RefBase {
 Timer::Timer() : impl_(new Timer::Impl()) {}
 
 Timer::~Timer() {
-  if (IsRunning()) Stop();
+  if (IsRunning()) Stop(false);
 }
 
 bool Timer::Start(TimerHandler* handler, uint32_t time_in_secs) {
@@ -87,7 +88,7 @@ bool Timer::Start(TimerHandler* handler, uint32_t time_in_secs) {
   return impl_->Start(handler, time_in_secs);
 }
 
-void Timer::Stop() { impl_->Stop(); }
+void Timer::Stop(bool wait_for_exit) { impl_->Stop(wait_for_exit); }
 
 bool Timer::IsRunning() { return impl_->IsRunning(); }
 

libwvdrmengine/cdm/src/wv_content_decryption_module.cpp

@@ -18,7 +18,7 @@
 #include "wv_cdm_event_listener.h"
 
 namespace {
-const int kCdmPolicyTimerDurationSeconds = 1;
+const int kCdmTimerDurationSeconds = 1;
 }
 
 namespace wvcdm {
@@ -28,8 +28,10 @@ std::mutex WvContentDecryptionModule::session_sharing_id_generation_lock_;
 WvContentDecryptionModule::WvContentDecryptionModule() {}
 
 WvContentDecryptionModule::~WvContentDecryptionModule() {
+  CryptoSession::DisableDelayedTermination();
   CloseAllCdms();
-  DisablePolicyTimer();
+  CryptoSession::TryTerminate();
+  DisableTimerAndWaitForExit();
 }
 
 bool WvContentDecryptionModule::IsSupported(const std::string& init_data_type) {
@@ -123,16 +125,9 @@ CdmResponseType WvContentDecryptionModule::GenerateKeyRequest(
   sts = cdm_engine->GenerateKeyRequest(session_id, key_set_id,
                                        initialization_data, license_type,
                                        app_parameters, key_request);
-  switch (license_type) {
-    case kLicenseTypeRelease:
-      if (sts != KEY_MESSAGE) {
-        cdm_engine->CloseKeySetSession(key_set_id);
-        cdm_by_session_id_.erase(key_set_id);
-      }
-      break;
-    default:
-      if (sts == KEY_MESSAGE) EnablePolicyTimer();
-      break;
+  if (license_type == kLicenseTypeRelease && sts != KEY_MESSAGE) {
+    cdm_engine->CloseKeySetSession(key_set_id);
+    cdm_by_session_id_.erase(key_set_id);
   }
   return sts;
 }
@@ -165,7 +160,6 @@ CdmResponseType WvContentDecryptionModule::RestoreKey(
   if (!cdm_engine) return SESSION_NOT_FOUND_4;
   CdmResponseType sts;
   sts = cdm_engine->RestoreKey(session_id, key_set_id);
-  if (sts == KEY_ADDED) EnablePolicyTimer();
   return sts;
 }
 
@@ -355,21 +349,28 @@ WvContentDecryptionModule::CdmInfo::CdmInfo()
 
 CdmEngine* WvContentDecryptionModule::EnsureCdmForIdentifier(
     const CdmIdentifier& identifier) {
-  std::unique_lock<std::mutex> auto_lock(cdms_lock_);
-  if (cdms_.find(identifier) == cdms_.end()) {
-    // Accessing the map entry will create a new instance using the default
-    // constructor. We then need to provide it with two pieces of info: The
-    // origin provided by the app and an identifier that uniquely identifies
-    // this CDM. We concatenate all pieces of the CdmIdentifier in order to
-    // create an ID that is unique to that identifier.
-    cdms_[identifier].file_system.set_origin(identifier.origin);
-    cdms_[identifier].file_system.set_identifier(identifier.spoid +
-                                                 identifier.origin);
-    cdms_[identifier].cdm_engine->SetAppPackageName(
-        identifier.app_package_name);
-    cdms_[identifier].cdm_engine->SetSpoid(identifier.spoid);
+  CdmEngine* cdm_engine;
+  bool enable_timer = false;
+  {
+    std::unique_lock<std::mutex> auto_lock(cdms_lock_);
+    if (cdms_.size() == 0) enable_timer = true;
+    if (cdms_.find(identifier) == cdms_.end()) {
+      // Accessing the map entry will create a new instance using the default
+      // constructor. We then need to provide it with two pieces of info: The
+      // origin provided by the app and an identifier that uniquely identifies
+      // this CDM. We concatenate all pieces of the CdmIdentifier in order to
+      // create an ID that is unique to that identifier.
+      cdms_[identifier].file_system.set_origin(identifier.origin);
+      cdms_[identifier].file_system.set_identifier(identifier.spoid +
+                                                   identifier.origin);
+      cdms_[identifier].cdm_engine->SetAppPackageName(
+          identifier.app_package_name);
+      cdms_[identifier].cdm_engine->SetSpoid(identifier.spoid);
+    }
+    cdm_engine = cdms_[identifier].cdm_engine.get();
   }
-  CdmEngine* cdm_engine = cdms_[identifier].cdm_engine.get();
+  // Do not enable timer while holding on to the |cdms_lock_|
+  if (enable_timer) EnableTimer();
 
   return cdm_engine;
 }
@@ -392,35 +393,23 @@ void WvContentDecryptionModule::CloseAllCdms() {
 
 CdmResponseType WvContentDecryptionModule::CloseCdm(
     const CdmIdentifier& cdm_identifier) {
-  // The policy timer ultimately calls OnTimerEvent (which wants to
-  // acquire cdms_lock_). Therefore, we cannot acquire cdms_lock_ and then the
-  // policy_timer_lock_ (via DisablePolicyTimer) at the same time.
-  // Acquire the cdms_lock_ first, in its own scope.
-  bool cdms_empty = false;
-  {
-    std::unique_lock<std::mutex> auto_lock(cdms_lock_);
-    auto it = cdms_.find(cdm_identifier);
-    if (it == cdms_.end()) {
-      LOGE("WVContentDecryptionModule::Close. cdm_identifier not found.");
-      // TODO(blueeyes): Create a better error.
-      return UNKNOWN_ERROR;
-    }
-    // Remove any sessions that point to this engine.
-    for (auto session_it = cdm_by_session_id_.begin();
-         session_it != cdm_by_session_id_.end();) {
-      if (session_it->second == it->second.cdm_engine.get()) {
-        session_it = cdm_by_session_id_.erase(session_it);
-      } else {
-        ++session_it;
-      }
-    }
-    cdms_.erase(it);
-    cdms_empty = cdms_.empty();
+  std::unique_lock<std::mutex> auto_lock(cdms_lock_);
+  auto it = cdms_.find(cdm_identifier);
+  if (it == cdms_.end()) {
+    LOGE("WVContentDecryptionModule::Close. cdm_identifier not found.");
+    // TODO(blueeyes): Create a better error.
+    return UNKNOWN_ERROR;
   }
-
-  if (cdms_empty) {
-    DisablePolicyTimer();
+  // Remove any sessions that point to this engine.
+  for (auto session_it = cdm_by_session_id_.begin();
+       session_it != cdm_by_session_id_.end();) {
+    if (session_it->second == it->second.cdm_engine.get()) {
+      session_it = cdm_by_session_id_.erase(session_it);
+    } else {
+      ++session_it;
+    }
   }
+  cdms_.erase(it);
   return NO_ERROR;
 }
 
@@ -462,23 +451,60 @@ CdmResponseType WvContentDecryptionModule::GetDecryptHashError(
   return cdm_engine->GetDecryptHashError(session_id, hash_error_string);
 }
 
-void WvContentDecryptionModule::EnablePolicyTimer() {
-  std::unique_lock<std::mutex> auto_lock(policy_timer_lock_);
-  if (!policy_timer_.IsRunning())
-    policy_timer_.Start(this, kCdmPolicyTimerDurationSeconds);
+void WvContentDecryptionModule::EnableTimer() {
+  std::unique_lock<std::mutex> auto_lock(timer_lock_);
+  if (!timer_.IsRunning()) timer_.Start(this, kCdmTimerDurationSeconds);
 }
 
-void WvContentDecryptionModule::DisablePolicyTimer() {
-  std::unique_lock<std::mutex> auto_lock(policy_timer_lock_);
-  if (policy_timer_.IsRunning()) {
-    policy_timer_.Stop();
+void WvContentDecryptionModule::DisableTimer() {
+  std::unique_lock<std::mutex> auto_lock(timer_lock_);
+  if (timer_.IsRunning()) {
+    timer_.Stop(false);
+  }
+}
+
+void WvContentDecryptionModule::DisableTimerAndWaitForExit() {
+  std::unique_lock<std::mutex> auto_lock(timer_lock_);
+  if (timer_.IsRunning()) {
+    timer_.Stop(true);
   }
 }
 
 void WvContentDecryptionModule::OnTimerEvent() {
-  std::unique_lock<std::mutex> auto_lock(cdms_lock_);
-  for (auto it = cdms_.begin(); it != cdms_.end(); ++it) {
-    it->second.cdm_engine->OnTimerEvent();
+  bool disable_timer = false;
+  {
+    std::unique_lock<std::mutex> auto_lock(cdms_lock_);
+    for (auto it = cdms_.begin(); it != cdms_.end(); ++it) {
+      it->second.cdm_engine->OnTimerEvent();
+    }
+    if (cdms_.empty()) {
+      if (CryptoSession::TryTerminate()) {
+        // If CryptoSession is in a state to be terminated, try acquiring the
+        // |timer_lock_| before deciding whether to disable the timer. If the
+        // lock cannot be acquired, there is no need to disable the timer.
+        // The |timer_lock_| is either being held by
+        // * EnableTimer - This does not appear possible, but if it did
+        //   happen |OnTimerEvent| will be called again. The timer can be
+        //   disabled during a future call.
+        // * DisableTimer - If so, allow that call to disable the timer. No
+        //   need to call to disable it here.
+        // * DisableTimerAndWaitForExit - If so, allow that call to disable
+        //   the timer. No need to call to disable it here. Note that
+        //   |DisableTimerAndWaitForExit| will try to stop the timer but
+        //   wait for it to exit. This might kick off the timer thread one
+        //   last time, which will call into OnTimerEvent. Calling
+        //   |DisableTimer| here will result in deadlock.
+        if (timer_lock_.try_lock()) {
+          disable_timer = true;
+          timer_lock_.unlock();
+        }
+      }
+    }
+  }
+
+  // Release |cdms_lock_| before attempting to disable the timer
+  if (disable_timer) {
+    DisableTimer();
   }
 }
 

libwvdrmengine/cdm/test/timer_unittest.cpp

@@ -8,17 +8,46 @@
 
 namespace wvcdm {
 
+namespace {
+
+// duration in seconds after starting the timer
+constexpr uint32_t kTestDuration = 5;
+// duration in seconds after test completes, before evaluating the result
+constexpr uint32_t kPostTestDuration = 3;
+
+}  // namespace
+
 class TestTimerHandler : public TimerHandler {
  public:
-  TestTimerHandler() : timer_events_(0){};
+  TestTimerHandler() : timer_events_count_(0){};
   virtual ~TestTimerHandler(){};
 
-  virtual void OnTimerEvent() { timer_events_++; }
+  virtual void OnTimerEvent() { timer_events_count_++; }
 
-  uint32_t timer_events() { return timer_events_; }
+  uint32_t timer_events_count() { return timer_events_count_; }
 
- private:
-  uint32_t timer_events_;
+ protected:
+  uint32_t timer_events_count_;
+};
+
+class TestStopTimerHandler : public TestTimerHandler {
+ public:
+  TestStopTimerHandler(uint32_t stop_at_timer_events_count, Timer* timer)
+      : stop_at_timer_events_count_(stop_at_timer_events_count),
+        timer_(timer) {}
+  virtual ~TestStopTimerHandler() {}
+
+  virtual void OnTimerEvent() {
+    if (++timer_events_count_ >= stop_at_timer_events_count_) {
+      if (timer_ != nullptr) {
+        timer_->Stop(false);
+      }
+    }
+  }
+
+ protected:
+  uint32_t stop_at_timer_events_count_;
+  Timer* timer_;
 };
 
 TEST(TimerTest, ParametersCheck) {
@@ -29,25 +58,58 @@ TEST(TimerTest, ParametersCheck) {
   EXPECT_FALSE(timer.Start(&handler, 0));
 }
 
-TEST(TimerTest, TimerCheck) {
+TEST(TimerTest, TimerCheck_StopAndWaitForExit) {
   TestTimerHandler handler;
   Timer timer;
-  uint32_t duration = 10;
 
-  EXPECT_EQ(0u, handler.timer_events());
+  EXPECT_EQ(0u, handler.timer_events_count());
   EXPECT_FALSE(timer.IsRunning());
 
   EXPECT_TRUE(timer.Start(&handler, 1));
   EXPECT_TRUE(timer.IsRunning());
-  sleep(duration);
+  sleep(kTestDuration);
 
-  EXPECT_LE(duration - 1, handler.timer_events());
-  EXPECT_LE(handler.timer_events(), duration + 1);
-  timer.Stop();
+  EXPECT_LE(kTestDuration - 1, handler.timer_events_count());
+  EXPECT_LE(handler.timer_events_count(), kTestDuration + 1);
+  timer.Stop(true);
   EXPECT_FALSE(timer.IsRunning());
-  sleep(duration);
+  sleep(kPostTestDuration);
 
-  EXPECT_LE(duration - 1, handler.timer_events());
-  EXPECT_LE(handler.timer_events(), duration + 1);
+  EXPECT_LE(kTestDuration - 1, handler.timer_events_count());
+  EXPECT_LE(handler.timer_events_count(), kTestDuration + 1);
 }
+
+TEST(TimerTest, TimerCheck_Stop) {
+  TestTimerHandler handler;
+  Timer timer;
+
+  EXPECT_EQ(0u, handler.timer_events_count());
+  EXPECT_FALSE(timer.IsRunning());
+
+  EXPECT_TRUE(timer.Start(&handler, 1));
+  EXPECT_TRUE(timer.IsRunning());
+  sleep(kTestDuration);
+
+  EXPECT_LE(kTestDuration - 1, handler.timer_events_count());
+  EXPECT_LE(handler.timer_events_count(), kTestDuration + 1);
+  timer.Stop(false);
+  sleep(kPostTestDuration);
+
+  EXPECT_FALSE(timer.IsRunning());
+}
+
+TEST(TimerTest, StopOnTimerThread) {
+  Timer timer;
+  TestStopTimerHandler handler(kTestDuration - 2, &timer);
+
+  EXPECT_EQ(0u, handler.timer_events_count());
+  EXPECT_FALSE(timer.IsRunning());
+
+  EXPECT_TRUE(timer.Start(&handler, 1));
+  EXPECT_TRUE(timer.IsRunning());
+  sleep(kTestDuration + kPostTestDuration);
+
+  EXPECT_FALSE(timer.IsRunning());
+}
+
 }  // namespace wvcdm