cashew: prevent nested byte counter deletion

When a byte counter is deleted, the byte counter makes a final stats
report to its delegate. It's possible for the callback handler for this
report to decide to delete the byte counter, causing a nested deletion.

Repro steps for this issue are to connect to 3G and then disable mobile
data from the internet settings page.

This CL prevents this nested deletion by changing
DataPlan::OnByteCounterUpdate not to stop the byte counter if there are
no active plans.

This CL also fixes some minor cpplint errors.

BUG=chromium-os:20144
TEST=unit tests and manual testing on device

Change-Id: I82998f5a610b13be808251b36934d3b7c29bad0d
Reviewed-on: http://gerrit.chromium.org/gerrit/10282
Tested-by: Vince Laviano <vlaviano@chromium.org>
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Commit-Ready: Vince Laviano <vlaviano@chromium.org>

src/aggregator_impl.h

diff --git a/src/aggregator_impl.h b/src/aggregator_impl.h
index e49bbed..9697efe 100644
--- a/src/aggregator_impl.h
+++ b/src/aggregator_impl.h
@@ -49,7 +49,6 @@
     virtual BytesPerDayRep GetBytesPerDay();
 
   private:
-
     // have I been successfully initialized via Init()?
     bool initialized_;
 

src/data_plan.cc

diff --git a/src/data_plan.cc b/src/data_plan.cc
index d4c3068..183422e 100644
--- a/src/data_plan.cc
+++ b/src/data_plan.cc
@@ -384,7 +384,6 @@
   DataPlan *active_plan = DataPlan::GetActivePlan(*data_plans);
   if (active_plan == NULL) {
     LOG(WARNING) << "OnByteCounterUpdate: no active plan";
-    device->StopByteCounter();
     return false;
   }
 

src/data_plan_unittest.cc

diff --git a/src/data_plan_unittest.cc b/src/data_plan_unittest.cc
index 80afcca..439f284 100644
--- a/src/data_plan_unittest.cc
+++ b/src/data_plan_unittest.cc
@@ -631,19 +631,15 @@
 
 // tests for local byte counter maintenance across plan transitions
 
-// empty list, we should stop the counter
+// empty list, call should fail
 TEST_F(DataPlanTest, NoActivePlans) {
   // create empty list
   DataPlanList data_plans;
   ASSERT_TRUE(data_plans.empty());
 
   // set up mock expectations and method return values
-  {
-    testing::InSequence expect_calls_in_order;
-    EXPECT_CALL(device_mock_, ByteCounterRunning())
-        .WillRepeatedly(testing::Return(true));
-    EXPECT_CALL(device_mock_, StopByteCounter()).Times(1);
-  }
+  EXPECT_CALL(device_mock_, ByteCounterRunning())
+      .WillRepeatedly(testing::Return(true));
 
   // simulate a byte counter update
   uint64 rx_bytes = 25 * kBytesPerMegabyte;
@@ -654,7 +650,6 @@
                                               tx_bytes);
 
   // check if test passed
-  // failure will most likely manifest as violated mock expectation
   EXPECT_FALSE(result);
 }
 

src/http_fetcher.h

diff --git a/src/http_fetcher.h b/src/http_fetcher.h
index 72a3dc9..06d07bb 100644
--- a/src/http_fetcher.h
+++ b/src/http_fetcher.h
@@ -80,6 +80,7 @@
 
   // The delegate; may be NULL.
   HttpFetcherDelegate* delegate_;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(HttpFetcher);
 };

src/libcurl_http_fetcher.h

diff --git a/src/libcurl_http_fetcher.h b/src/libcurl_http_fetcher.h
index 24ee547..b8179b4 100644
--- a/src/libcurl_http_fetcher.h
+++ b/src/libcurl_http_fetcher.h
@@ -13,6 +13,7 @@
 
 #include <map>
 #include <string>
+#include <utility>
 
 #include <base/basictypes.h>  // NOLINT
 

src/procfs_byte_counter.cc

diff --git a/src/procfs_byte_counter.cc b/src/procfs_byte_counter.cc
index f709bc6..bffb5a6 100644
--- a/src/procfs_byte_counter.cc
+++ b/src/procfs_byte_counter.cc
@@ -56,7 +56,7 @@
 ProcfsByteCounter::~ProcfsByteCounter() {
   DCHECK(proc_net_dev_fp_ == NULL);
   if (read_stats_source_id_ != 0 && !g_source_remove(read_stats_source_id_)) {
-    LOG(WARNING) << interface_ << ": dtor: g_source_remove_failed";
+    LOG(WARNING) << interface_ << ": dtor: g_source_remove failed";
   }
   // read and update delegate with most recent stats
   if (!UpdateDelegateWithStats()) {