From 8c5dc514bfca9af8b7b03797f7e4c88e0ee89381 Mon Sep 17 00:00:00 2001 From: Paul Hu Date: Thu, 18 Jul 2024 06:11:44 +0000 Subject: [PATCH] Update the QueryTaskConfig constructor This is a no-op refactor. Move some parameters from QueryTaskConfig to QueryTask, or bring them from public methods. Bug: 345146854 Test: atest FrameworksNetTests NsdManagerTest Change-Id: I47d420842c4f9cec3fe25b5e2febc84a1e495f40 --- .../connectivity/mdns/MdnsQueryScheduler.java | 26 ++++++---- .../mdns/MdnsServiceTypeClient.java | 34 +++++++++----- .../connectivity/mdns/QueryTaskConfig.java | 47 ++++++------------- .../mdns/MdnsServiceTypeClientTests.java | 18 +++---- 4 files changed, 60 insertions(+), 65 deletions(-) diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsQueryScheduler.java b/service-t/src/com/android/server/connectivity/mdns/MdnsQueryScheduler.java index 3fcf0d4b45..5c02767f78 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsQueryScheduler.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsQueryScheduler.java @@ -63,17 +63,22 @@ public class MdnsQueryScheduler { * rescheduling is not necessary. */ @Nullable - public ScheduledQueryTaskArgs maybeRescheduleCurrentRun(long now, - long minRemainingTtl, long lastSentTime, long sessionId) { + public ScheduledQueryTaskArgs maybeRescheduleCurrentRun( + long now, + long minRemainingTtl, + long lastSentTime, + long sessionId, + int numOfQueriesBeforeBackoff) { if (mLastScheduledQueryTaskArgs == null) { return null; } - if (!mLastScheduledQueryTaskArgs.config.shouldUseQueryBackoff()) { + if (!mLastScheduledQueryTaskArgs.config.shouldUseQueryBackoff(numOfQueriesBeforeBackoff)) { return null; } final long timeToRun = calculateTimeToRun(mLastScheduledQueryTaskArgs, - mLastScheduledQueryTaskArgs.config, now, minRemainingTtl, lastSentTime); + mLastScheduledQueryTaskArgs.config, now, minRemainingTtl, lastSentTime, + numOfQueriesBeforeBackoff); if (timeToRun <= mLastScheduledQueryTaskArgs.timeToRun) { return null; @@ -95,14 +100,16 @@ public class MdnsQueryScheduler { long minRemainingTtl, long now, long lastSentTime, - long sessionId) { - final QueryTaskConfig nextRunConfig = currentConfig.getConfigForNextRun(); + long sessionId, + int queryMode, + int numOfQueriesBeforeBackoff) { + final QueryTaskConfig nextRunConfig = currentConfig.getConfigForNextRun(queryMode); final long timeToRun; if (mLastScheduledQueryTaskArgs == null) { timeToRun = now + nextRunConfig.delayUntilNextTaskWithoutBackoffMs; } else { timeToRun = calculateTimeToRun(mLastScheduledQueryTaskArgs, - nextRunConfig, now, minRemainingTtl, lastSentTime); + nextRunConfig, now, minRemainingTtl, lastSentTime, numOfQueriesBeforeBackoff); } mLastScheduledQueryTaskArgs = new ScheduledQueryTaskArgs(nextRunConfig, timeToRun, minRemainingTtl + now, @@ -122,9 +129,10 @@ public class MdnsQueryScheduler { } private static long calculateTimeToRun(@NonNull ScheduledQueryTaskArgs taskArgs, - QueryTaskConfig queryTaskConfig, long now, long minRemainingTtl, long lastSentTime) { + QueryTaskConfig queryTaskConfig, long now, long minRemainingTtl, long lastSentTime, + int numOfQueriesBeforeBackoff) { final long baseDelayInMs = queryTaskConfig.delayUntilNextTaskWithoutBackoffMs; - if (!queryTaskConfig.shouldUseQueryBackoff()) { + if (!queryTaskConfig.shouldUseQueryBackoff(numOfQueriesBeforeBackoff)) { return lastSentTime + baseDelayInMs; } if (minRemainingTtl <= 0) { diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java index 643430ab88..5f9a9e7b57 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java @@ -147,7 +147,8 @@ public class MdnsServiceTypeClient { final List servicesToResolve = makeResponsesForResolve(socketKey); final QueryTask queryTask = new QueryTask(taskArgs, servicesToResolve, getAllDiscoverySubtypes(), needSendDiscoveryQueries(listeners), - getExistingServices()); + getExistingServices(), searchOptions.onlyUseIpv6OnIpv6OnlyNetworks(), + socketKey); executor.submit(queryTask); break; } @@ -178,7 +179,9 @@ public class MdnsServiceTypeClient { minRemainingTtl, now, lastSentTime, - sentResult.taskArgs.sessionId + sentResult.taskArgs.sessionId, + searchOptions.getQueryMode(), + searchOptions.numOfQueriesBeforeBackoff() ); dependencies.sendMessageDelayed( handler, @@ -396,10 +399,7 @@ public class MdnsServiceTypeClient { // Keep tracking the ScheduledFuture for the task so we can cancel it if caller is not // interested anymore. final QueryTaskConfig taskConfig = new QueryTaskConfig( - searchOptions.getQueryMode(), - searchOptions.onlyUseIpv6OnIpv6OnlyNetworks(), - searchOptions.numOfQueriesBeforeBackoff(), - socketKey); + searchOptions.getQueryMode()); final long now = clock.elapsedRealtime(); if (lastSentTime == 0) { lastSentTime = now; @@ -412,7 +412,9 @@ public class MdnsServiceTypeClient { minRemainingTtl, now, lastSentTime, - currentSessionId + currentSessionId, + searchOptions.getQueryMode(), + searchOptions.numOfQueriesBeforeBackoff() ); dependencies.sendMessageDelayed( handler, @@ -424,7 +426,8 @@ public class MdnsServiceTypeClient { mdnsQueryScheduler.scheduleFirstRun(taskConfig, now, minRemainingTtl, currentSessionId), servicesToResolve, getAllDiscoverySubtypes(), needSendDiscoveryQueries(listeners), - getExistingServices()); + getExistingServices(), searchOptions.onlyUseIpv6OnIpv6OnlyNetworks(), + socketKey); executor.submit(queryTask); } @@ -535,7 +538,8 @@ public class MdnsServiceTypeClient { final long minRemainingTtl = getMinRemainingTtl(now); MdnsQueryScheduler.ScheduledQueryTaskArgs args = mdnsQueryScheduler.maybeRescheduleCurrentRun(now, minRemainingTtl, - lastSentTime, currentSessionId + 1); + lastSentTime, currentSessionId + 1, + searchOptions.numOfQueriesBeforeBackoff()); if (args != null) { removeScheduledTask(); dependencies.sendMessageDelayed( @@ -723,15 +727,21 @@ public class MdnsServiceTypeClient { private final List subtypes = new ArrayList<>(); private final boolean sendDiscoveryQueries; private final List existingServices = new ArrayList<>(); + private final boolean onlyUseIpv6OnIpv6OnlyNetworks; + private final SocketKey socketKey; QueryTask(@NonNull MdnsQueryScheduler.ScheduledQueryTaskArgs taskArgs, @NonNull Collection servicesToResolve, @NonNull Collection subtypes, boolean sendDiscoveryQueries, - @NonNull Collection existingServices) { + @NonNull Collection existingServices, + boolean onlyUseIpv6OnIpv6OnlyNetworks, + @NonNull SocketKey socketKey) { this.taskArgs = taskArgs; this.servicesToResolve.addAll(servicesToResolve); this.subtypes.addAll(subtypes); this.sendDiscoveryQueries = sendDiscoveryQueries; this.existingServices.addAll(existingServices); + this.onlyUseIpv6OnIpv6OnlyNetworks = onlyUseIpv6OnIpv6OnlyNetworks; + this.socketKey = socketKey; } @Override @@ -745,8 +755,8 @@ public class MdnsServiceTypeClient { subtypes, taskArgs.config.expectUnicastResponse, taskArgs.config.transactionId, - taskArgs.config.socketKey, - taskArgs.config.onlyUseIpv6OnIpv6OnlyNetworks, + socketKey, + onlyUseIpv6OnIpv6OnlyNetworks, sendDiscoveryQueries, servicesToResolve, clock, diff --git a/service-t/src/com/android/server/connectivity/mdns/QueryTaskConfig.java b/service-t/src/com/android/server/connectivity/mdns/QueryTaskConfig.java index 089416655f..d2cd46348d 100644 --- a/service-t/src/com/android/server/connectivity/mdns/QueryTaskConfig.java +++ b/service-t/src/com/android/server/connectivity/mdns/QueryTaskConfig.java @@ -19,9 +19,6 @@ package com.android.server.connectivity.mdns; import static com.android.server.connectivity.mdns.MdnsSearchOptions.AGGRESSIVE_QUERY_MODE; import static com.android.server.connectivity.mdns.MdnsSearchOptions.PASSIVE_QUERY_MODE; -import android.annotation.NonNull; -import android.annotation.Nullable; - import com.android.internal.annotations.VisibleForTesting; /** @@ -51,9 +48,6 @@ public class QueryTaskConfig { static final int MAX_TIME_BETWEEN_AGGRESSIVE_BURSTS_MS = 60000; private final boolean alwaysAskForUnicastResponse = MdnsConfigs.alwaysAskForUnicastResponseInEachBurst(); - private final int queryMode; - final boolean onlyUseIpv6OnIpv6OnlyNetworks; - private final int numOfQueriesBeforeBackoff; @VisibleForTesting final int transactionId; @VisibleForTesting @@ -64,16 +58,11 @@ public class QueryTaskConfig { final long delayUntilNextTaskWithoutBackoffMs; private final boolean isFirstBurst; private final long queryCount; - @NonNull - final SocketKey socketKey; - QueryTaskConfig(@NonNull QueryTaskConfig other, long queryCount, int transactionId, + QueryTaskConfig(long queryCount, int transactionId, boolean expectUnicastResponse, boolean isFirstBurst, int burstCounter, int queriesPerBurst, int timeBetweenBurstsInMs, long delayUntilNextTaskWithoutBackoffMs) { - this.queryMode = other.queryMode; - this.onlyUseIpv6OnIpv6OnlyNetworks = other.onlyUseIpv6OnIpv6OnlyNetworks; - this.numOfQueriesBeforeBackoff = other.numOfQueriesBeforeBackoff; this.transactionId = transactionId; this.expectUnicastResponse = expectUnicastResponse; this.queriesPerBurst = queriesPerBurst; @@ -82,27 +71,20 @@ public class QueryTaskConfig { this.delayUntilNextTaskWithoutBackoffMs = delayUntilNextTaskWithoutBackoffMs; this.isFirstBurst = isFirstBurst; this.queryCount = queryCount; - this.socketKey = other.socketKey; } - QueryTaskConfig(int queryMode, - boolean onlyUseIpv6OnIpv6OnlyNetworks, - int numOfQueriesBeforeBackoff, - @Nullable SocketKey socketKey) { - this.queryMode = queryMode; - this.onlyUseIpv6OnIpv6OnlyNetworks = onlyUseIpv6OnIpv6OnlyNetworks; - this.numOfQueriesBeforeBackoff = numOfQueriesBeforeBackoff; + QueryTaskConfig(int queryMode) { this.queriesPerBurst = QUERIES_PER_BURST; this.burstCounter = 0; this.transactionId = 1; this.expectUnicastResponse = true; this.isFirstBurst = true; // Config the scan frequency based on the scan mode. - if (this.queryMode == AGGRESSIVE_QUERY_MODE) { + if (queryMode == AGGRESSIVE_QUERY_MODE) { this.timeBetweenBurstsInMs = INITIAL_AGGRESSIVE_TIME_BETWEEN_BURSTS_MS; this.delayUntilNextTaskWithoutBackoffMs = TIME_BETWEEN_RETRANSMISSION_QUERIES_IN_BURST_MS; - } else if (this.queryMode == PASSIVE_QUERY_MODE) { + } else if (queryMode == PASSIVE_QUERY_MODE) { // In passive scan mode, sends a single burst of QUERIES_PER_BURST queries, and then // in each TIME_BETWEEN_BURSTS interval, sends QUERIES_PER_BURST_PASSIVE_MODE // queries. @@ -116,12 +98,11 @@ public class QueryTaskConfig { this.timeBetweenBurstsInMs = INITIAL_TIME_BETWEEN_BURSTS_MS; this.delayUntilNextTaskWithoutBackoffMs = TIME_BETWEEN_QUERIES_IN_BURST_MS; } - this.socketKey = socketKey; this.queryCount = 0; } long getDelayUntilNextTaskWithoutBackoff(boolean isFirstQueryInBurst, - boolean isLastQueryInBurst) { + boolean isLastQueryInBurst, int queryMode) { if (isFirstQueryInBurst && queryMode == AGGRESSIVE_QUERY_MODE) { return 0; } @@ -133,7 +114,7 @@ public class QueryTaskConfig { : TIME_BETWEEN_QUERIES_IN_BURST_MS; } - boolean getNextExpectUnicastResponse(boolean isLastQueryInBurst) { + boolean getNextExpectUnicastResponse(boolean isLastQueryInBurst, int queryMode) { if (!isLastQueryInBurst) { return false; } @@ -143,7 +124,7 @@ public class QueryTaskConfig { return alwaysAskForUnicastResponse; } - int getNextTimeBetweenBurstsMs(boolean isLastQueryInBurst) { + int getNextTimeBetweenBurstsMs(boolean isLastQueryInBurst, int queryMode) { if (!isLastQueryInBurst) { return timeBetweenBurstsInMs; } @@ -155,7 +136,7 @@ public class QueryTaskConfig { /** * Get new QueryTaskConfig for next run. */ - public QueryTaskConfig getConfigForNextRun() { + public QueryTaskConfig getConfigForNextRun(int queryMode) { long newQueryCount = queryCount + 1; int newTransactionId = transactionId + 1; if (newTransactionId > UNSIGNED_SHORT_MAX_VALUE) { @@ -177,16 +158,18 @@ public class QueryTaskConfig { } } - return new QueryTaskConfig(this, newQueryCount, newTransactionId, - getNextExpectUnicastResponse(isLastQueryInBurst), newIsFirstBurst, newBurstCounter, - newQueriesPerBurst, getNextTimeBetweenBurstsMs(isLastQueryInBurst), - getDelayUntilNextTaskWithoutBackoff(isFirstQueryInBurst, isLastQueryInBurst)); + return new QueryTaskConfig(newQueryCount, newTransactionId, + getNextExpectUnicastResponse(isLastQueryInBurst, queryMode), newIsFirstBurst, + newBurstCounter, newQueriesPerBurst, + getNextTimeBetweenBurstsMs(isLastQueryInBurst, queryMode), + getDelayUntilNextTaskWithoutBackoff( + isFirstQueryInBurst, isLastQueryInBurst, queryMode)); } /** * Determine if the query backoff should be used. */ - public boolean shouldUseQueryBackoff() { + public boolean shouldUseQueryBackoff(int numOfQueriesBeforeBackoff) { // Don't enable backoff mode during the burst or in the first burst if (burstCounter != 0 || isFirstBurst) { return false; diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java index 44fa55cd05..569f4d7823 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java @@ -562,10 +562,7 @@ public class MdnsServiceTypeClientTests { //MdnsConfigsFlagsImpl.alwaysAskForUnicastResponseInEachBurst.override(true); MdnsSearchOptions searchOptions = MdnsSearchOptions.newBuilder() .addSubtype(SUBTYPE).setQueryMode(ACTIVE_QUERY_MODE).build(); - QueryTaskConfig config = new QueryTaskConfig( - searchOptions.getQueryMode(), - false /* onlyUseIpv6OnIpv6OnlyNetworks */, 3 /* numOfQueriesBeforeBackoff */, - socketKey); + QueryTaskConfig config = new QueryTaskConfig(searchOptions.getQueryMode()); // This is the first query. We will ask for unicast response. assertTrue(config.expectUnicastResponse); @@ -574,14 +571,14 @@ public class MdnsServiceTypeClientTests { // For the rest of queries in this burst, we will NOT ask for unicast response. for (int i = 1; i < MdnsConfigs.queriesPerBurst(); i++) { int oldTransactionId = config.transactionId; - config = config.getConfigForNextRun(); + config = config.getConfigForNextRun(ACTIVE_QUERY_MODE); assertFalse(config.expectUnicastResponse); assertEquals(config.transactionId, oldTransactionId + 1); } // This is the first query of a new burst. We will ask for unicast response. int oldTransactionId = config.transactionId; - config = config.getConfigForNextRun(); + config = config.getConfigForNextRun(ACTIVE_QUERY_MODE); assertTrue(config.expectUnicastResponse); assertEquals(config.transactionId, oldTransactionId + 1); } @@ -590,10 +587,7 @@ public class MdnsServiceTypeClientTests { public void testQueryTaskConfig_askForUnicastInFirstQuery() { MdnsSearchOptions searchOptions = MdnsSearchOptions.newBuilder() .addSubtype(SUBTYPE).setQueryMode(ACTIVE_QUERY_MODE).build(); - QueryTaskConfig config = new QueryTaskConfig( - searchOptions.getQueryMode(), - false /* onlyUseIpv6OnIpv6OnlyNetworks */, 3 /* numOfQueriesBeforeBackoff */, - socketKey); + QueryTaskConfig config = new QueryTaskConfig(searchOptions.getQueryMode()); // This is the first query. We will ask for unicast response. assertTrue(config.expectUnicastResponse); @@ -602,14 +596,14 @@ public class MdnsServiceTypeClientTests { // For the rest of queries in this burst, we will NOT ask for unicast response. for (int i = 1; i < MdnsConfigs.queriesPerBurst(); i++) { int oldTransactionId = config.transactionId; - config = config.getConfigForNextRun(); + config = config.getConfigForNextRun(ACTIVE_QUERY_MODE); assertFalse(config.expectUnicastResponse); assertEquals(config.transactionId, oldTransactionId + 1); } // This is the first query of a new burst. We will NOT ask for unicast response. int oldTransactionId = config.transactionId; - config = config.getConfigForNextRun(); + config = config.getConfigForNextRun(ACTIVE_QUERY_MODE); assertFalse(config.expectUnicastResponse); assertEquals(config.transactionId, oldTransactionId + 1); }