Revert of New method StatsObserver::OnCompleteReports, passing ownership. (patchset #2 id:20001 of https://codereview.webrtc.org/2584553002/ )
Reason for revert:
The new method doesn't work as intended.
It can't pass ownership, because the StatsReports is a vector of raw pointers to StatReport objects owned by the StatsCollector.
Original issue's description:
> New method StatsObserver::OnCompleteReports, passing ownership.
>
> The new name, OnCompleteReports rather than OnComplete, is needed
> because in C++ method lookup, overriding a method hides all otherwise
> inherited methods with the same name, even if they have a different
> signature. And here, the intention is that each subclass should
> override one or the other of the two methods, and inherit the method it
> doesn't override.
>
> This cl is a prerequisite for
> https://codereview.webrtc.org/2567143003/, because the Chrome glue
> code needs to retain the stats report after the OnComplete method has
> returned.
>
> Currently, Chrome makes a copy of the stats mapping (which breaks when
> changing ValuePtr from an rtc::linked_ptr to an std::unique_ptr). After
> this cl, Chrome can be fixed to take ownership and no longer needs to
> copy anything, unblocking cl 2567143003.
>
> BUG=webrtc:6424
>
> Review-Url: https://codereview.webrtc.org/2584553002
> Cr-Commit-Position: refs/heads/master@{#15708}
> Committed: b36ee8d498
TBR=solenberg@webrtc.org,magjed@webrtc.org,tkchin@webrtc.org,hbos@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:6424
Review-Url: https://codereview.webrtc.org/2641783002
Cr-Commit-Position: refs/heads/master@{#16144}
This commit is contained in:
@ -1606,9 +1606,9 @@ void PeerConnection::OnMessage(rtc::Message* msg) {
|
|||||||
}
|
}
|
||||||
case MSG_GETSTATS: {
|
case MSG_GETSTATS: {
|
||||||
GetStatsMsg* param = static_cast<GetStatsMsg*>(msg->pdata);
|
GetStatsMsg* param = static_cast<GetStatsMsg*>(msg->pdata);
|
||||||
std::unique_ptr<StatsReports> reports(new StatsReports);
|
StatsReports reports;
|
||||||
stats_->GetStats(param->track, reports.get());
|
stats_->GetStats(param->track, &reports);
|
||||||
param->observer->OnCompleteReports(std::move(reports));
|
param->observer->OnComplete(reports);
|
||||||
delete param;
|
delete param;
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|||||||
@ -109,14 +109,7 @@ class StreamCollectionInterface : public rtc::RefCountInterface {
|
|||||||
|
|
||||||
class StatsObserver : public rtc::RefCountInterface {
|
class StatsObserver : public rtc::RefCountInterface {
|
||||||
public:
|
public:
|
||||||
// TODO(nisse, hbos): Old version, not passing ownership. Should
|
virtual void OnComplete(const StatsReports& reports) = 0;
|
||||||
// perhaps be deprecated, but since all of this is a legacy
|
|
||||||
// interface anyway, probably best to leave as is until this class
|
|
||||||
// can be deleted.
|
|
||||||
virtual void OnComplete(const StatsReports& reports) {}
|
|
||||||
virtual void OnCompleteReports(std::unique_ptr<StatsReports> reports) {
|
|
||||||
OnComplete(*reports);
|
|
||||||
}
|
|
||||||
|
|
||||||
protected:
|
protected:
|
||||||
virtual ~StatsObserver() {}
|
virtual ~StatsObserver() {}
|
||||||
|
|||||||
@ -108,12 +108,12 @@ class MockStatsObserver : public webrtc::StatsObserver {
|
|||||||
MockStatsObserver() : called_(false), stats_() {}
|
MockStatsObserver() : called_(false), stats_() {}
|
||||||
virtual ~MockStatsObserver() {}
|
virtual ~MockStatsObserver() {}
|
||||||
|
|
||||||
void OnCompleteReports(std::unique_ptr<StatsReports> reports) override {
|
virtual void OnComplete(const StatsReports& reports) {
|
||||||
ASSERT(!called_);
|
ASSERT(!called_);
|
||||||
called_ = true;
|
called_ = true;
|
||||||
stats_.Clear();
|
stats_.Clear();
|
||||||
stats_.number_of_reports = reports->size();
|
stats_.number_of_reports = reports.size();
|
||||||
for (const auto* r : *reports) {
|
for (const auto* r : reports) {
|
||||||
if (r->type() == StatsReport::kStatsReportTypeSsrc) {
|
if (r->type() == StatsReport::kStatsReportTypeSsrc) {
|
||||||
stats_.timestamp = r->timestamp();
|
stats_.timestamp = r->timestamp();
|
||||||
GetIntValue(r, StatsReport::kStatsValueNameAudioOutputLevel,
|
GetIntValue(r, StatsReport::kStatsValueNameAudioOutputLevel,
|
||||||
|
|||||||
@ -740,9 +740,9 @@ class StatsObserverWrapper : public StatsObserver {
|
|||||||
|
|
||||||
virtual ~StatsObserverWrapper() {}
|
virtual ~StatsObserverWrapper() {}
|
||||||
|
|
||||||
void OnCompleteReports(std::unique_ptr<StatsReports> reports) override {
|
void OnComplete(const StatsReports& reports) override {
|
||||||
ScopedLocalRefFrame local_ref_frame(jni());
|
ScopedLocalRefFrame local_ref_frame(jni());
|
||||||
jobjectArray j_reports = ReportsToJava(jni(), *reports);
|
jobjectArray j_reports = ReportsToJava(jni(), reports);
|
||||||
jmethodID m = GetMethodID(jni(), *j_observer_class_, "onComplete",
|
jmethodID m = GetMethodID(jni(), *j_observer_class_, "onComplete",
|
||||||
"([Lorg/webrtc/StatsReport;)V");
|
"([Lorg/webrtc/StatsReport;)V");
|
||||||
jni()->CallVoidMethod(*j_observer_global_, m, j_reports);
|
jni()->CallVoidMethod(*j_observer_global_, m, j_reports);
|
||||||
|
|||||||
@ -29,10 +29,10 @@ class StatsObserverAdapter : public StatsObserver {
|
|||||||
completion_handler_ = nil;
|
completion_handler_ = nil;
|
||||||
}
|
}
|
||||||
|
|
||||||
void OnCompleteReports(std::unique_ptr<StatsReports> reports) override {
|
void OnComplete(const StatsReports& reports) override {
|
||||||
RTC_DCHECK(completion_handler_);
|
RTC_DCHECK(completion_handler_);
|
||||||
NSMutableArray *stats = [NSMutableArray arrayWithCapacity:reports->size()];
|
NSMutableArray *stats = [NSMutableArray arrayWithCapacity:reports.size()];
|
||||||
for (const auto* report : *reports) {
|
for (const auto* report : reports) {
|
||||||
RTCLegacyStatsReport *statsReport =
|
RTCLegacyStatsReport *statsReport =
|
||||||
[[RTCLegacyStatsReport alloc] initWithNativeReport:*report];
|
[[RTCLegacyStatsReport alloc] initWithNativeReport:*report];
|
||||||
[stats addObject:statsReport];
|
[stats addObject:statsReport];
|
||||||
|
|||||||
Reference in New Issue
Block a user