Replace blocking invokes with PostTask in AndroidNetworkMonitor

Use PendingTaskSafetyFlag for safe Stop. Followup to
https://webrtc-review.googlesource.com/c/src/+/209181.

Also fix rtc::scoped_refptr to work with RTC_PT_GUARDED_BY.

Bug: webrtc:12339
Change-Id: Ic0e3ecb17049f1a0e6af887ba5f97a5b48a32d98
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/211351
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Reviewed-by: Taylor <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33447}
This commit is contained in:
Niels Möller
2021-03-12 14:08:23 +01:00
committed by Commit Bot
parent 048e9c2f45
commit 662b306bae
5 changed files with 41 additions and 14 deletions

View File

@ -104,6 +104,7 @@ class scoped_refptr {
T* get() const { return ptr_; } T* get() const { return ptr_; }
operator T*() const { return ptr_; } operator T*() const { return ptr_; }
T& operator*() const { return *ptr_; }
T* operator->() const { return ptr_; } T* operator->() const { return ptr_; }
// Returns the (possibly null) raw pointer, and makes the scoped_refptr hold a // Returns the (possibly null) raw pointer, and makes the scoped_refptr hold a

View File

@ -141,7 +141,10 @@ if (is_android) {
suppressed_configs += [ "//build/config/android:hide_all_but_jni_onload" ] suppressed_configs += [ "//build/config/android:hide_all_but_jni_onload" ]
configs += [ "//build/config/android:hide_all_but_jni" ] configs += [ "//build/config/android:hide_all_but_jni" ]
ldflags = [ "-lEGL", "-Wl,--build-id" ] ldflags = [
"-lEGL",
"-Wl,--build-id",
]
deps = [ deps = [
":libjingle_peerconnection_jni", ":libjingle_peerconnection_jni",
@ -565,6 +568,8 @@ if (current_os == "linux" || is_android) {
"../../rtc_base:ip_address", "../../rtc_base:ip_address",
"../../rtc_base:rtc_base_approved", "../../rtc_base:rtc_base_approved",
"../../rtc_base:threading", "../../rtc_base:threading",
"../../rtc_base/task_utils:pending_task_safety_flag",
"../../rtc_base/task_utils:to_queued_task",
"../../system_wrappers:field_trial", "../../system_wrappers:field_trial",
"../../system_wrappers:metrics", "../../system_wrappers:metrics",
] ]

View File

@ -51,11 +51,16 @@ class AndroidNetworkMonitorTest : public ::testing::Test {
std::make_unique<jni::AndroidNetworkMonitor>(env, context); std::make_unique<jni::AndroidNetworkMonitor>(env, context);
} }
void SetUp() { void SetUp() override {
// Reset network monitor states. // Reset network monitor states.
network_monitor_->Stop(); network_monitor_->Stop();
} }
void TearDown() override {
// The network monitor must be stopped, before it is destructed.
network_monitor_->Stop();
}
protected: protected:
std::unique_ptr<jni::AndroidNetworkMonitor> network_monitor_; std::unique_ptr<jni::AndroidNetworkMonitor> network_monitor_;
}; };

View File

@ -21,6 +21,7 @@
#include "rtc_base/ip_address.h" #include "rtc_base/ip_address.h"
#include "rtc_base/logging.h" #include "rtc_base/logging.h"
#include "rtc_base/strings/string_builder.h" #include "rtc_base/strings/string_builder.h"
#include "rtc_base/task_utils/to_queued_task.h"
#include "sdk/android/generated_base_jni/NetworkChangeDetector_jni.h" #include "sdk/android/generated_base_jni/NetworkChangeDetector_jni.h"
#include "sdk/android/generated_base_jni/NetworkMonitor_jni.h" #include "sdk/android/generated_base_jni/NetworkMonitor_jni.h"
#include "sdk/android/native_api/jni/java_types.h" #include "sdk/android/native_api/jni/java_types.h"
@ -228,9 +229,12 @@ AndroidNetworkMonitor::AndroidNetworkMonitor(
: android_sdk_int_(Java_NetworkMonitor_androidSdkInt(env)), : android_sdk_int_(Java_NetworkMonitor_androidSdkInt(env)),
j_application_context_(env, j_application_context), j_application_context_(env, j_application_context),
j_network_monitor_(env, Java_NetworkMonitor_getInstance(env)), j_network_monitor_(env, Java_NetworkMonitor_getInstance(env)),
network_thread_(rtc::Thread::Current()) {} network_thread_(rtc::Thread::Current()),
safety_flag_(PendingTaskSafetyFlag::Create()) {}
AndroidNetworkMonitor::~AndroidNetworkMonitor() = default; AndroidNetworkMonitor::~AndroidNetworkMonitor() {
RTC_DCHECK(!started_);
}
void AndroidNetworkMonitor::Start() { void AndroidNetworkMonitor::Start() {
RTC_DCHECK_RUN_ON(network_thread_); RTC_DCHECK_RUN_ON(network_thread_);
@ -246,6 +250,9 @@ void AndroidNetworkMonitor::Start() {
bind_using_ifname_ = bind_using_ifname_ =
webrtc::field_trial::IsEnabled("WebRTC-BindUsingInterfaceName"); webrtc::field_trial::IsEnabled("WebRTC-BindUsingInterfaceName");
// Needed for restart after Stop().
safety_flag_->SetAlive();
JNIEnv* env = AttachCurrentThreadIfNeeded(); JNIEnv* env = AttachCurrentThreadIfNeeded();
Java_NetworkMonitor_startMonitoring( Java_NetworkMonitor_startMonitoring(
env, j_network_monitor_, j_application_context_, jlongFromPointer(this)); env, j_network_monitor_, j_application_context_, jlongFromPointer(this));
@ -259,6 +266,10 @@ void AndroidNetworkMonitor::Stop() {
started_ = false; started_ = false;
find_network_handle_without_ipv6_temporary_part_ = false; find_network_handle_without_ipv6_temporary_part_ = false;
// Cancel any pending tasks. We should not call SignalNetworksChanged when the
// monitor is stopped.
safety_flag_->SetNotAlive();
JNIEnv* env = AttachCurrentThreadIfNeeded(); JNIEnv* env = AttachCurrentThreadIfNeeded();
Java_NetworkMonitor_stopMonitoring(env, j_network_monitor_, Java_NetworkMonitor_stopMonitoring(env, j_network_monitor_,
jlongFromPointer(this)); jlongFromPointer(this));
@ -571,11 +582,11 @@ AndroidNetworkMonitorFactory::CreateNetworkMonitor() {
void AndroidNetworkMonitor::NotifyConnectionTypeChanged( void AndroidNetworkMonitor::NotifyConnectionTypeChanged(
JNIEnv* env, JNIEnv* env,
const JavaRef<jobject>& j_caller) { const JavaRef<jobject>& j_caller) {
network_thread_->Invoke<void>(RTC_FROM_HERE, [this] { network_thread_->PostTask(ToQueuedTask(safety_flag_, [this] {
RTC_LOG(LS_INFO) RTC_LOG(LS_INFO)
<< "Android network monitor detected connection type change."; << "Android network monitor detected connection type change.";
SignalNetworksChanged(); SignalNetworksChanged();
}); }));
} }
void AndroidNetworkMonitor::NotifyOfActiveNetworkList( void AndroidNetworkMonitor::NotifyOfActiveNetworkList(
@ -594,18 +605,19 @@ void AndroidNetworkMonitor::NotifyOfNetworkConnect(
const JavaRef<jobject>& j_network_info) { const JavaRef<jobject>& j_network_info) {
NetworkInformation network_info = NetworkInformation network_info =
GetNetworkInformationFromJava(env, j_network_info); GetNetworkInformationFromJava(env, j_network_info);
network_thread_->Invoke<void>(RTC_FROM_HERE, [this, &network_info] { network_thread_->PostTask(ToQueuedTask(
safety_flag_, [this, network_info = std::move(network_info)] {
OnNetworkConnected_n(network_info); OnNetworkConnected_n(network_info);
}); }));
} }
void AndroidNetworkMonitor::NotifyOfNetworkDisconnect( void AndroidNetworkMonitor::NotifyOfNetworkDisconnect(
JNIEnv* env, JNIEnv* env,
const JavaRef<jobject>& j_caller, const JavaRef<jobject>& j_caller,
jlong network_handle) { jlong network_handle) {
network_thread_->Invoke<void>(RTC_FROM_HERE, [this, network_handle] { network_thread_->PostTask(ToQueuedTask(safety_flag_, [this, network_handle] {
OnNetworkDisconnected_n(static_cast<NetworkHandle>(network_handle)); OnNetworkDisconnected_n(static_cast<NetworkHandle>(network_handle));
}); }));
} }
void AndroidNetworkMonitor::NotifyOfNetworkPreference( void AndroidNetworkMonitor::NotifyOfNetworkPreference(
@ -617,9 +629,9 @@ void AndroidNetworkMonitor::NotifyOfNetworkPreference(
rtc::NetworkPreference preference = rtc::NetworkPreference preference =
static_cast<rtc::NetworkPreference>(jpreference); static_cast<rtc::NetworkPreference>(jpreference);
network_thread_->Invoke<void>(RTC_FROM_HERE, [this, type, preference] { network_thread_->PostTask(ToQueuedTask(
OnNetworkPreference_n(type, preference); safety_flag_,
}); [this, type, preference] { OnNetworkPreference_n(type, preference); }));
} }
} // namespace jni } // namespace jni

View File

@ -12,6 +12,7 @@
#define SDK_ANDROID_SRC_JNI_ANDROID_NETWORK_MONITOR_H_ #define SDK_ANDROID_SRC_JNI_ANDROID_NETWORK_MONITOR_H_
#include <stdint.h> #include <stdint.h>
#include <map> #include <map>
#include <string> #include <string>
#include <vector> #include <vector>
@ -19,6 +20,7 @@
#include "absl/types/optional.h" #include "absl/types/optional.h"
#include "rtc_base/network_monitor.h" #include "rtc_base/network_monitor.h"
#include "rtc_base/network_monitor_factory.h" #include "rtc_base/network_monitor_factory.h"
#include "rtc_base/task_utils/pending_task_safety_flag.h"
#include "rtc_base/thread.h" #include "rtc_base/thread.h"
#include "rtc_base/thread_annotations.h" #include "rtc_base/thread_annotations.h"
#include "sdk/android/src/jni/jni_helpers.h" #include "sdk/android/src/jni/jni_helpers.h"
@ -147,6 +149,8 @@ class AndroidNetworkMonitor : public rtc::NetworkMonitorInterface {
// This applies to adapter_type_by_name_, vpn_underlying_adapter_type_by_name_ // This applies to adapter_type_by_name_, vpn_underlying_adapter_type_by_name_
// and FindNetworkHandleFromIfname. // and FindNetworkHandleFromIfname.
bool bind_using_ifname_ RTC_GUARDED_BY(network_thread_) = true; bool bind_using_ifname_ RTC_GUARDED_BY(network_thread_) = true;
const rtc::scoped_refptr<PendingTaskSafetyFlag> safety_flag_
RTC_PT_GUARDED_BY(network_thread_);
}; };
class AndroidNetworkMonitorFactory : public rtc::NetworkMonitorFactory { class AndroidNetworkMonitorFactory : public rtc::NetworkMonitorFactory {