A step towards changing StatsReport::Value::name to an enum.
The stats reporting code does a lot of unnecessary string copying.
This is a step in the direction of removing that and forcing use of only known constants.
This is a reland of an already reviewed cl that got reverted by mistake.
TBR=xians@google.com
Review URL: https://webrtc-codereview.appspot.com/12989004
git-svn-id: http://webrtc.googlecode.com/svn/trunk/talk@6678 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/app/webrtc/objc/RTCStatsReport.mm b/app/webrtc/objc/RTCStatsReport.mm
index 8da4b46..98cf654 100644
--- a/app/webrtc/objc/RTCStatsReport.mm
+++ b/app/webrtc/objc/RTCStatsReport.mm
@@ -57,7 +57,7 @@
[NSMutableArray arrayWithCapacity:statsReport.values.size()];
webrtc::StatsReport::Values::const_iterator it = statsReport.values.begin();
for (; it != statsReport.values.end(); ++it) {
- RTCPair* pair = [[RTCPair alloc] initWithKey:@(it->name.c_str())
+ RTCPair* pair = [[RTCPair alloc] initWithKey:@(it->display_name())
value:@(it->value.c_str())];
[values addObject:pair];
}
diff --git a/app/webrtc/statscollector.cc b/app/webrtc/statscollector.cc
index b80da23..dd615ab 100644
--- a/app/webrtc/statscollector.cc
+++ b/app/webrtc/statscollector.cc
@@ -193,19 +193,17 @@
const char StatsReport::kStatsReportVideoBweId[] = "bweforvideo";
// Implementations of functions in statstypes.h
-void StatsReport::AddValue(const std::string& name, const std::string& value) {
- Value temp;
- temp.name = name;
- temp.value = value;
- values.push_back(temp);
+void StatsReport::AddValue(StatsReport::StatsValueName name,
+ const std::string& value) {
+ values.push_back(Value(name, value));
}
-void StatsReport::AddValue(const std::string& name, int64 value) {
+void StatsReport::AddValue(StatsReport::StatsValueName name, int64 value) {
AddValue(name, talk_base::ToString<int64>(value));
}
template <typename T>
-void StatsReport::AddValue(const std::string& name,
+void StatsReport::AddValue(StatsReport::StatsValueName name,
const std::vector<T>& value) {
std::ostringstream oss;
oss << "[";
@@ -218,11 +216,11 @@
AddValue(name, oss.str());
}
-void StatsReport::AddBoolean(const std::string& name, bool value) {
+void StatsReport::AddBoolean(StatsReport::StatsValueName name, bool value) {
AddValue(name, value ? "true" : "false");
}
-void StatsReport::ReplaceValue(const std::string& name,
+void StatsReport::ReplaceValue(StatsReport::StatsValueName name,
const std::string& value) {
for (Values::iterator it = values.begin(); it != values.end(); ++it) {
if ((*it).name == name) {
@@ -258,7 +256,7 @@
bool ExtractValueFromReport(
const StatsReport& report,
- const std::string& name,
+ StatsReport::StatsValueName name,
std::string* value) {
StatsReport::Values::const_iterator it = report.values.begin();
for (; it != report.values.end(); ++it) {
diff --git a/app/webrtc/statscollector_unittest.cc b/app/webrtc/statscollector_unittest.cc
index 76251e2..fc0ca0e 100644
--- a/app/webrtc/statscollector_unittest.cc
+++ b/app/webrtc/statscollector_unittest.cc
@@ -149,7 +149,7 @@
};
bool GetValue(const StatsReport* report,
- const std::string& name,
+ StatsReport::StatsValueName name,
std::string* value) {
StatsReport::Values::const_iterator it = report->values.begin();
for (; it != report->values.end(); ++it) {
@@ -163,7 +163,7 @@
std::string ExtractStatsValue(const std::string& type,
const StatsReports& reports,
- const std::string name) {
+ StatsReport::StatsValueName name) {
if (reports.empty()) {
return kNoReports;
}
@@ -204,13 +204,13 @@
}
std::string ExtractSsrcStatsValue(StatsReports reports,
- const std::string& name) {
+ StatsReport::StatsValueName name) {
return ExtractStatsValue(
StatsReport::kStatsReportTypeSsrc, reports, name);
}
std::string ExtractBweStatsValue(StatsReports reports,
- const std::string& name) {
+ StatsReport::StatsValueName name) {
return ExtractStatsValue(
StatsReport::kStatsReportTypeBwe, reports, name);
}
@@ -446,7 +446,7 @@
// This creates a standard setup with a transport called "trspname"
// having one transport channel
// and the specified virtual connection name.
- void InitSessionStats(const std::string vc_name) {
+ void InitSessionStats(const std::string& vc_name) {
const std::string kTransportName("trspname");
cricket::TransportStats transport_stats;
cricket::TransportChannelStats channel_stats;
@@ -687,11 +687,12 @@
EXPECT_CALL(session_, video_channel()).WillRepeatedly(Return(&video_channel));
EXPECT_CALL(session_, voice_channel()).WillRepeatedly(ReturnNull());
EXPECT_CALL(*media_channel, GetStats(_, _))
- .WillOnce(DoAll(SetArgPointee<1>(stats_read),
- Return(true)));
+ .WillOnce(DoAll(SetArgPointee<1>(stats_read),
+ Return(true)));
stats.UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard);
stats.GetStats(NULL, &reports);
- std::string result = ExtractSsrcStatsValue(reports, "bytesSent");
+ std::string result = ExtractSsrcStatsValue(reports,
+ StatsReport::kStatsValueNameBytesSent);
EXPECT_EQ(kBytesSentString, result);
}
@@ -730,9 +731,11 @@
stats.UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard);
stats.GetStats(NULL, &reports);
- std::string result = ExtractSsrcStatsValue(reports, "bytesSent");
+ std::string result = ExtractSsrcStatsValue(reports,
+ StatsReport::kStatsValueNameBytesSent);
EXPECT_EQ(kBytesSentString, result);
- result = ExtractBweStatsValue(reports, "googTargetEncBitrate");
+ result = ExtractBweStatsValue(reports,
+ StatsReport::kStatsValueNameTargetEncBitrate);
EXPECT_EQ(kTargetEncBitrateString, result);
}
@@ -850,6 +853,9 @@
// stats object, and that this transport stats object exists in stats.
TEST_F(StatsCollectorTest, TransportObjectLinkedFromSsrcObject) {
webrtc::StatsCollector stats(&session_); // Implementation under test.
+ // Ignore unused callback (logspam).
+ EXPECT_CALL(session_, GetTransport(_))
+ .WillOnce(Return(static_cast<cricket::Transport*>(NULL)));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
// The content_name known by the video channel.
const std::string kVcName("vcname");
@@ -919,6 +925,9 @@
// an outgoing SSRC where stats are returned.
TEST_F(StatsCollectorTest, RemoteSsrcInfoIsPresent) {
webrtc::StatsCollector stats(&session_); // Implementation under test.
+ // Ignore unused callback (logspam).
+ EXPECT_CALL(session_, GetTransport(_))
+ .WillOnce(Return(static_cast<cricket::Transport*>(NULL)));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
// The content_name known by the video channel.
const std::string kVcName("vcname");
@@ -1190,13 +1199,15 @@
StatsReports reports; // returned values.
stats.GetStats(NULL, &reports);
std::string result = ExtractBweStatsValue(
- reports, "googReceivedPacketGroupPropagationDeltaSumDebug");
+ reports,
+ StatsReport::kStatsValueNameRecvPacketGroupPropagationDeltaSumDebug);
EXPECT_EQ("10", result);
result = ExtractBweStatsValue(
- reports, "googReceivedPacketGroupPropagationDeltaDebug");
+ reports,
+ StatsReport::kStatsValueNameRecvPacketGroupPropagationDeltaDebug);
EXPECT_EQ("[100, 200]", result);
result = ExtractBweStatsValue(
- reports, "googReceivedPacketGroupArrivalTimeDebug");
+ reports, StatsReport::kStatsValueNameRecvPacketGroupArrivalTimeDebug);
EXPECT_EQ("[1000, 2000]", result);
}
@@ -1204,6 +1215,10 @@
// AudioTrackInterface::GetStats() method.
TEST_F(StatsCollectorTest, GetStatsFromLocalAudioTrack) {
webrtc::StatsCollector stats(&session_); // Implementation under test.
+ // Ignore unused callback (logspam).
+ EXPECT_CALL(session_, GetTransport(_))
+ .WillOnce(Return(static_cast<cricket::Transport*>(NULL)));
+
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
// The content_name known by the voice channel.
const std::string kVcName("vcname");
@@ -1233,6 +1248,9 @@
// correctly.
TEST_F(StatsCollectorTest, GetStatsFromRemoteStream) {
webrtc::StatsCollector stats(&session_); // Implementation under test.
+ // Ignore unused callback (logspam).
+ EXPECT_CALL(session_, GetTransport(_))
+ .WillOnce(Return(static_cast<cricket::Transport*>(NULL)));
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
// The content_name known by the voice channel.
const std::string kVcName("vcname");
@@ -1256,6 +1274,9 @@
// after a RemoveLocalAudioTrack() call.
TEST_F(StatsCollectorTest, GetStatsAfterRemoveAudioStream) {
webrtc::StatsCollector stats(&session_); // Implementation under test.
+ // Ignore unused callback (logspam).
+ EXPECT_CALL(session_, GetTransport(_))
+ .WillOnce(Return(static_cast<cricket::Transport*>(NULL)));
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
// The content_name known by the voice channel.
const std::string kVcName("vcname");
@@ -1310,6 +1331,9 @@
// the same ssrc, they populate stats reports correctly.
TEST_F(StatsCollectorTest, LocalAndRemoteTracksWithSameSsrc) {
webrtc::StatsCollector stats(&session_); // Implementation under test.
+ // Ignore unused callback (logspam).
+ EXPECT_CALL(session_, GetTransport(_))
+ .WillOnce(Return(static_cast<cricket::Transport*>(NULL)));
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
// The content_name known by the voice channel.
const std::string kVcName("vcname");
@@ -1388,6 +1412,9 @@
// avoid duplication of code in test cases.
TEST_F(StatsCollectorTest, TwoLocalTracksWithSameSsrc) {
webrtc::StatsCollector stats(&session_); // Implementation under test.
+ // Ignore unused callback (logspam).
+ EXPECT_CALL(session_, GetTransport(_))
+ .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
// The content_name known by the voice channel.
const std::string kVcName("vcname");
diff --git a/app/webrtc/statstypes.h b/app/webrtc/statstypes.h
index 0a2aa03..828b9f5 100644
--- a/app/webrtc/statstypes.h
+++ b/app/webrtc/statstypes.h
@@ -43,26 +43,55 @@
public:
StatsReport() : timestamp(0) { }
+ // TODO(tommi): Change this to be an enum type that holds all the
+ // kStatsValueName constants.
+ typedef const char* StatsValueName;
+
std::string id; // See below for contents.
std::string type; // See below for contents.
struct Value {
- std::string name;
+ Value() : name(NULL) {}
+ // The copy ctor can't be declared as explicit due to problems with STL.
+ Value(const Value& other) : name(other.name), value(other.value) {}
+ explicit Value(StatsValueName name) : name(name) {}
+ Value(StatsValueName name, const std::string& value)
+ : name(name), value(value) {
+ }
+
+ // TODO(tommi): Remove this operator once we don't need it.
+ // The operator is provided for compatibility with STL containers.
+ // The public |name| member variable is otherwise meant to be read-only.
+ Value& operator=(const Value& other) {
+ const_cast<StatsValueName&>(name) = other.name;
+ value = other.value;
+ return *this;
+ }
+
+ // TODO(tommi): Change implementation to do a simple enum value-to-static-
+ // string conversion when client code has been updated to use this method
+ // instead of the |name| member variable.
+ const char* display_name() const { return name; }
+
+ const StatsValueName name;
+
std::string value;
};
- void AddValue(const std::string& name, const std::string& value);
- void AddValue(const std::string& name, int64 value);
+ void AddValue(StatsValueName name, const std::string& value);
+ void AddValue(StatsValueName name, int64 value);
template <typename T>
- void AddValue(const std::string& name, const std::vector<T>& value);
- void AddBoolean(const std::string& name, bool value);
+ void AddValue(StatsValueName name, const std::vector<T>& value);
+ void AddBoolean(StatsValueName name, bool value);
- void ReplaceValue(const std::string& name, const std::string& value);
+ void ReplaceValue(StatsValueName name, const std::string& value);
double timestamp; // Time since 1970-01-01T00:00:00Z in milliseconds.
typedef std::vector<Value> Values;
Values values;
+ // TODO(tommi): These should all be enum values.
+
// StatsReport types.
// A StatsReport of |type| = "googSession" contains overall information
// about the thing libjingle calls a session (which may contain one
