Skip to content

Commit

Permalink
[Jank] Add TraceEvents to two networking scenarios.
Browse files Browse the repository at this point in the history 
1) DataUseManager has an IPC parent that's 12th on scroll jank items.
   Speculating on slow Android framework calls from here.
2) While investigating this, saw slow trace events from Android
   NetworkCallback that lacked visibility.

Bug: 1287653
Change-Id: Id49b059bd1150825621c39e047a63e16be8a442d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3677076
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: Yaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1010285}
  • Loading branch information
@yfriedman
yfriedman authored and Chromium LUCI CQ committed Jun 2, 2022
1 parent 15c12a7 commit eaa152b
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 67 deletions.
3 changes: 3 additions & 0 deletions chrome/browser/data_use_measurement/chrome_data_use_measurement.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h"
#include "base/no_destructor.h"
#include "base/trace_event/trace_event.h"
#include "content/public/browser/browser_thread.h"

#if BUILDFLAG(IS_ANDROID)
Expand All @@ -27,6 +28,7 @@ ChromeDataUseMeasurement& ChromeDataUseMeasurement::GetInstance() {

ChromeDataUseMeasurement::ChromeDataUseMeasurement() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
TRACE_EVENT0("browser", "ChromeDataUseMeasurement");

#if BUILDFLAG(IS_ANDROID)
int64_t bytes = 0;
Expand Down Expand Up @@ -59,6 +61,7 @@ void ChromeDataUseMeasurement::ReportNetworkServiceDataUse(
void ChromeDataUseMeasurement::ReportDataUsage(TrafficDirection dir,
int64_t message_size_bytes) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
TRACE_EVENT0("browser", "ChromeDataUseMeasurement::ReportDataUsage");

if (message_size_bytes <= 0)
return;
Expand Down
144 changes: 77 additions & 67 deletions net/android/java/src/org/chromium/net/NetworkChangeNotifierAutoDetect.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.chromium.base.ApplicationStatus;
import org.chromium.base.ContextUtils;
import org.chromium.base.StrictModeContext;
import org.chromium.base.TraceEvent;
import org.chromium.base.compat.ApiHelperForM;
import org.chromium.base.compat.ApiHelperForO;
import org.chromium.base.compat.ApiHelperForP;
Expand Down Expand Up @@ -744,95 +745,104 @@ private boolean ignoreConnectedNetwork(Network network, NetworkCapabilities capa

@Override
public void onAvailable(Network network) {
final NetworkCapabilities capabilities =
mConnectivityManagerDelegate.getNetworkCapabilities(network);
if (ignoreConnectedNetwork(network, capabilities)) {
return;
}
final boolean makeVpnDefault = capabilities.hasTransport(TRANSPORT_VPN) &&
// Only make the VPN the default if it isn't already.
(mVpnInPlace == null || !network.equals(mVpnInPlace));
if (makeVpnDefault) {
mVpnInPlace = network;
}
final long netId = networkToNetId(network);
@ConnectionType
final int connectionType = mConnectivityManagerDelegate.getConnectionType(network);
runOnThread(new Runnable() {
@Override
public void run() {
mObserver.onNetworkConnect(netId, connectionType);
if (makeVpnDefault) {
// Make VPN the default network.
mObserver.onConnectionTypeChanged(connectionType);
// Purge all other networks as they're inaccessible to Chrome now.
mObserver.purgeActiveNetworkList(new long[] {netId});
}
try (TraceEvent e = TraceEvent.scoped("NetworkChangeNotifierCallback::onAvailable")) {
final NetworkCapabilities capabilities =
mConnectivityManagerDelegate.getNetworkCapabilities(network);
if (ignoreConnectedNetwork(network, capabilities)) {
return;
}
});
final boolean makeVpnDefault = capabilities.hasTransport(TRANSPORT_VPN) &&
// Only make the VPN the default if it isn't already.
(mVpnInPlace == null || !network.equals(mVpnInPlace));
if (makeVpnDefault) {
mVpnInPlace = network;
}
final long netId = networkToNetId(network);
@ConnectionType
final int connectionType = mConnectivityManagerDelegate.getConnectionType(network);
runOnThread(new Runnable() {
@Override
public void run() {
mObserver.onNetworkConnect(netId, connectionType);
if (makeVpnDefault) {
// Make VPN the default network.
mObserver.onConnectionTypeChanged(connectionType);
// Purge all other networks as they're inaccessible to Chrome now.
mObserver.purgeActiveNetworkList(new long[] {netId});
}
}
});
}
}

@Override
public void onCapabilitiesChanged(
Network network, NetworkCapabilities networkCapabilities) {
if (ignoreConnectedNetwork(network, networkCapabilities)) {
return;
}
// A capabilities change may indicate the ConnectionType has changed,
// so forward the new ConnectionType along to observer.
final long netId = networkToNetId(network);
final int connectionType = mConnectivityManagerDelegate.getConnectionType(network);
runOnThread(new Runnable() {
@Override
public void run() {
mObserver.onNetworkConnect(netId, connectionType);
try (TraceEvent e = TraceEvent.scoped(
"NetworkChangeNotifierCallback::onCapabilitiesChanged")) {
if (ignoreConnectedNetwork(network, networkCapabilities)) {
return;
}
});
// A capabilities change may indicate the ConnectionType has changed,
// so forward the new ConnectionType along to observer.
final long netId = networkToNetId(network);
final int connectionType = mConnectivityManagerDelegate.getConnectionType(network);
runOnThread(new Runnable() {
@Override
public void run() {
mObserver.onNetworkConnect(netId, connectionType);
}
});
}
}

@Override
public void onLosing(Network network, int maxMsToLive) {
if (ignoreConnectedNetwork(network, null)) {
return;
}
final long netId = networkToNetId(network);
runOnThread(new Runnable() {
@Override
public void run() {
mObserver.onNetworkSoonToDisconnect(netId);
try (TraceEvent e = TraceEvent.scoped("NetworkChangeNotifierCallback::onLosing")) {
if (ignoreConnectedNetwork(network, null)) {
return;
}
});
final long netId = networkToNetId(network);
runOnThread(new Runnable() {
@Override
public void run() {
mObserver.onNetworkSoonToDisconnect(netId);
}
});
}
}

@Override
public void onLost(final Network network) {
if (ignoreNetworkDueToVpn(network)) {
return;
}
runOnThread(new Runnable() {
@Override
public void run() {
mObserver.onNetworkDisconnect(networkToNetId(network));
}
});
// If the VPN is going away, inform observer that other networks that were previously
// hidden by ignoreNetworkDueToVpn() are now available for use, now that this user's
// traffic is not forced into the VPN.
if (mVpnInPlace != null) {
assert network.equals(mVpnInPlace);
mVpnInPlace = null;
for (Network newNetwork :
getAllNetworksFiltered(mConnectivityManagerDelegate, network)) {
onAvailable(newNetwork);
try (TraceEvent e = TraceEvent.scoped("NetworkChangeNotifierCallback::onLost")) {
if (ignoreNetworkDueToVpn(network)) {
return;
}
@ConnectionType
final int newConnectionType = getCurrentNetworkState().getConnectionType();
runOnThread(new Runnable() {
@Override
public void run() {
mObserver.onConnectionTypeChanged(newConnectionType);
mObserver.onNetworkDisconnect(networkToNetId(network));
}
});
// If the VPN is going away, inform observer that other networks that were
// previously hidden by ignoreNetworkDueToVpn() are now available for use, now that
// this user's traffic is not forced into the VPN.
if (mVpnInPlace != null) {
assert network.equals(mVpnInPlace);
mVpnInPlace = null;
for (Network newNetwork :
getAllNetworksFiltered(mConnectivityManagerDelegate, network)) {
onAvailable(newNetwork);
}
@ConnectionType
final int newConnectionType = getCurrentNetworkState().getConnectionType();
runOnThread(new Runnable() {
@Override
public void run() {
mObserver.onConnectionTypeChanged(newConnectionType);
}
});
}
}
}
}
Expand Down

0 comments on commit eaa152b

Please sign in to comment.