From ba9073d39cab25ba4ce22f019bebccdb4126f08e Mon Sep 17 00:00:00 2001
From: Philip Gregor <pgregorr@amazon.com>
Date: Wed, 3 Apr 2024 10:57:59 -0700
Subject: [PATCH] Addressed comments by bzbarsky-apple now using
 browseIdentifier to stop DNS-SD browse

---
 src/lib/dnssd/platform/Dnssd.h                |  1 -
 src/platform/android/DnssdImpl.cpp            | 11 +++++--
 src/platform/android/DnssdImpl.h              |  8 +++++
 .../platform/NsdManagerServiceBrowser.java    | 30 ++++++++++++++-----
 4 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/src/lib/dnssd/platform/Dnssd.h b/src/lib/dnssd/platform/Dnssd.h
index a5044a255f5aba..778e9bc05f1d9c 100644
--- a/src/lib/dnssd/platform/Dnssd.h
+++ b/src/lib/dnssd/platform/Dnssd.h
@@ -230,7 +230,6 @@ CHIP_ERROR ChipDnssdBrowse(const char * type, DnssdServiceProtocol protocol, chi
  *
  * @param browseIdentifier an identifier for a browse operation that came from
  *                         ChipDnssdBrowse.
- *
  */
 CHIP_ERROR ChipDnssdStopBrowse(intptr_t browseIdentifier);
 
diff --git a/src/platform/android/DnssdImpl.cpp b/src/platform/android/DnssdImpl.cpp
index a25ec585f985c5..5a27b76c7ebae1 100644
--- a/src/platform/android/DnssdImpl.cpp
+++ b/src/platform/android/DnssdImpl.cpp
@@ -204,17 +204,20 @@ CHIP_ERROR ChipDnssdBrowse(const char * type, DnssdServiceProtocol protocol, Ine
         return CHIP_JNI_ERROR_EXCEPTION_THROWN;
     }
 
-    *browseIdentifier = reinterpret_cast<intptr_t>(nullptr);
+    auto sdCtx        = chip::Platform::New<BrowseContext>(callback);
+    *browseIdentifier = reinterpret_cast<intptr_t>(sdCtx);
+
     return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR ChipDnssdStopBrowse(intptr_t browseIdentifier)
 {
-    VerifyOrReturnError(sMdnsCallbackObject.HasValidObjectRef(), CHIP_ERROR_INCORRECT_STATE);
+    VerifyOrReturnError(sBrowserObject.HasValidObjectRef() && sStopBrowseMethod != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
 
     JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
+    auto ctx     = reinterpret_cast<BrowseContext *>(browseIdentifier);
 
-    env->CallVoidMethod(sBrowserObject.ObjectRef(), sStopBrowseMethod, sMdnsCallbackObject.ObjectRef());
+    env->CallVoidMethod(sBrowserObject.ObjectRef(), sStopBrowseMethod, reinterpret_cast<jlong>(ctx->callback));
 
     if (env->ExceptionCheck())
     {
@@ -223,6 +226,8 @@ CHIP_ERROR ChipDnssdStopBrowse(intptr_t browseIdentifier)
         env->ExceptionClear();
         return CHIP_JNI_ERROR_EXCEPTION_THROWN;
     }
+    chip::Platform::Delete(ctx);
+
     return CHIP_NO_ERROR;
 }
 
diff --git a/src/platform/android/DnssdImpl.h b/src/platform/android/DnssdImpl.h
index a16015767fab90..f106ec6c1a9ce0 100644
--- a/src/platform/android/DnssdImpl.h
+++ b/src/platform/android/DnssdImpl.h
@@ -18,6 +18,7 @@
 #pragma once
 
 #include <jni.h>
+#include <lib/dnssd/platform/Dnssd.h>
 
 namespace chip {
 namespace Dnssd {
@@ -37,5 +38,12 @@ void HandleResolve(jstring instanceName, jstring serviceType, jstring hostName,
 
 void HandleBrowse(jobjectArray instanceName, jstring serviceType, jlong callbackHandle, jlong contextHandle);
 
+struct BrowseContext
+{
+    DnssdBrowseCallback callback;
+
+    BrowseContext(DnssdBrowseCallback cb) { callback = cb; };
+};
+
 } // namespace Dnssd
 } // namespace chip
diff --git a/src/platform/android/java/chip/platform/NsdManagerServiceBrowser.java b/src/platform/android/java/chip/platform/NsdManagerServiceBrowser.java
index a4cc225a4b2fbb..8963eec4763e28 100644
--- a/src/platform/android/java/chip/platform/NsdManagerServiceBrowser.java
+++ b/src/platform/android/java/chip/platform/NsdManagerServiceBrowser.java
@@ -69,6 +69,13 @@ public void browse(
         new Runnable() {
           @Override
           public void run() {
+            Log.i(
+                TAG,
+                "Browse for service '"
+                    + serviceType
+                    + "' expired after timeout: "
+                    + timeout
+                    + " ms");
             stopDiscover(callbackHandle);
           }
         };
@@ -82,7 +89,7 @@ public void startDiscover(
       final long contextHandle,
       final ChipMdnsCallback chipMdnsCallback) {
     if (callbackMap.containsKey(callbackHandle)) {
-      Log.d(TAG, "Invalid callbackHandle");
+      Log.w(TAG, "Starting service discovering failed. Invalid callback ID: " + callbackHandle);
       return;
     }
 
@@ -91,28 +98,35 @@ public void startDiscover(
     multicastLock.acquire();
     discovery.setChipMdnsCallback(chipMdnsCallback);
 
-    Log.d(TAG, "Starting service discovering for '" + serviceType + "'");
+    Log.d(
+        TAG,
+        "Starting service discovering for '"
+            + serviceType
+            + "' with callback ID: "
+            + callbackHandle);
 
     this.nsdManager.discoverServices(serviceType, NsdManager.PROTOCOL_DNS_SD, discovery);
     callbackMap.put(callbackHandle, discovery);
   }
 
   public void stopDiscover(final long callbackHandle) {
+    Log.d(TAG, "Stopping service discovering with callback ID: " + callbackHandle);
     if (!callbackMap.containsKey(callbackHandle)) {
+      Log.w(TAG, "Stopping service discovering failed. Callback handle not found.");
       return;
     }
 
-    MessageQueue queue = mainThreadHandler.getLooper().getQueue();
-    if (!queue.isIdle()) {
-      Log.i(TAG, "canceling scheduled browse timeout runnable");
-      mainThreadHandler.removeCallbacksAndMessages(null);
-    }
-
     NsdManagerDiscovery discovery = callbackMap.remove(callbackHandle);
     if (multicastLock.isHeld()) {
       multicastLock.release();
     }
 
+    MessageQueue queue = mainThreadHandler.getLooper().getQueue();
+    if (!queue.isIdle()) {
+      Log.d(TAG, "Canceling scheduled browse timeout runnable for '" + discovery.serviceType + "'");
+      mainThreadHandler.removeCallbacksAndMessages(null);
+    }
+
     this.nsdManager.stopServiceDiscovery(discovery);
   }