commit ceba6aa64b2b49b03f4a94faf9846d30a4fbfa9b
author Gary Morain <gmorain@chromium.org> Thu May 03 10:28:26 2012 -0700
committer Gerrit <chrome-bot@google.com> Fri May 04 14:59:44 2012 -0700
tree 7d17fe056e5a6ed472be00c4a25ba86039a822aa
parent a0caabfa2443efdff8ef87e6db0186b68133cea5

shill: Allow null callback in OnScanReply

The current implemtation of Cellular::Scan calls
CellularCapability::Scan with a null callback.
CellularCapabilityUniversal::OnScanReply now checks for null before
trying to run the callback.

BUG=None
TEST=manual.  Invoke a scan on a E362 modem and observe that the scan
compeltes without shill crashing.  Also added unittest.

Change-Id: I368d6b30e8b482c01015af6214546bd108daa796
Reviewed-on: https://gerrit.chromium.org/gerrit/21777
Commit-Ready: Gary Morain <gmorain@chromium.org>
Reviewed-by: Gary Morain <gmorain@chromium.org>
Tested-by: Gary Morain <gmorain@chromium.org>

cellular_capability_universal.cc

diff --git a/cellular_capability_universal.cc b/cellular_capability_universal.cc
index afe90b5..1a39ebb 100644
--- a/cellular_capability_universal.cc
+++ b/cellular_capability_universal.cc
@@ -664,7 +664,14 @@
   }
   cellular()->adaptor()->EmitStringmapsChanged(flimflam::kFoundNetworksProperty,
                                                found_networks_);
-  callback.Run(error);
+
+  // TODO(gmorain): This check for is_null() is a work-around because
+  // Cellular::Scan() passes a null callback.  Instead: 1. Have Cellular::Scan()
+  // pass in a callback. 2. Have Cellular "own" the found_networks_ property
+  // 3. Have Cellular EmitStingMapsChanged() 4. Share the code between GSM and
+  // Universal.
+  if (!callback.is_null())
+    callback.Run(error);
 }
 
 Stringmap CellularCapabilityUniversal::ParseScanResult(

cellular_capability_universal_unittest.cc

diff --git a/cellular_capability_universal_unittest.cc b/cellular_capability_universal_unittest.cc
index a8dd456..4cce549 100644
--- a/cellular_capability_universal_unittest.cc
+++ b/cellular_capability_universal_unittest.cc
@@ -116,6 +116,15 @@
   }
 
 
+  void InvokeScan(Error *error, const DBusPropertyMapsCallback &callback,
+                  int timeout) {
+    callback.Run(CellularCapabilityUniversal::ScanResults(), Error());
+  }
+
+  void Set3gppProxy() {
+    capability_->modem_3gpp_proxy_.reset(modem_3gpp_proxy_.release());
+  }
+
   MOCK_METHOD1(TestCallback, void(const Error &error));
 
  protected:
@@ -366,4 +375,21 @@
   EXPECT_EQ(kOperatorName, capability_->spn_);
 }
 
+MATCHER_P(SizeIs, value, "") {
+  return static_cast<size_t>(value) == arg.size();
+}
+
+// Validates that OnScanReply does not crash with a null callback.
+TEST_F(CellularCapabilityUniversalTest, ScanWithNullCallback) {
+  Error error;
+  EXPECT_CALL(*modem_3gpp_proxy_, Scan(_, _, CellularCapability::kTimeoutScan))
+      .WillOnce(Invoke(this, &CellularCapabilityUniversalTest::InvokeScan));
+  EXPECT_CALL(*device_adaptor_,
+              EmitStringmapsChanged(flimflam::kFoundNetworksProperty,
+                                    SizeIs(0)));
+  Set3gppProxy();
+  capability_->Scan(&error, ResultCallback());
+  EXPECT_TRUE(error.IsSuccess());
+}
+
 }  // namespace shill