Addressed comments by sharadb-amazon

examples/tv-casting-app/tv-casting-common/core/CastingPlayer.cpp

@@ -67,18 +67,14 @@ void CastingPlayer::VerifyOrEstablishConnection(ConnectionCallbacks connectionCa
                         "CommissionerDeclarationHandler");
         // Set the callback for handling CommissionerDeclaration messages.
         matter::casting::core::CommissionerDeclarationHandler::GetInstance()->SetCommissionerDeclarationCallback(
-            connectionCallbacks.mCommissionerDeclarationCallback, mTargetCastingPlayer);
+            connectionCallbacks.mCommissionerDeclarationCallback);
         mClientProvidedCommissionerDeclarationCallback = true;
     }
     else
     {
         ChipLogProgress(
             AppServer,
             "CastingPlayer::VerifyOrEstablishConnection() CommissionerDeclarationCallback not provided in ConnectionCallbacks");
-        // Still set the target casting player in the CommissionerDeclarationHandler in case the we receive a Commissioner
-        // Delaration message with CancelPasscode.
-        matter::casting::core::CommissionerDeclarationHandler::GetInstance()->SetCommissionerDeclarationCallback(
-            nullptr, mTargetCastingPlayer);
         mClientProvidedCommissionerDeclarationCallback = false;
     }
 
@@ -236,6 +232,10 @@ CHIP_ERROR CastingPlayer::StopConnecting()
         ChipLogError(AppServer,
                      "CastingPlayer::StopConnecting() mIdOptions.mCommissionerPasscode == false, ContinueConnecting() should only "
                      "be called when the CastingPlayer/Commissioner-Generated passcode commissioning flow is in progress."););
+    // Calling the internal StopConnecting() API with the shouldSendIdentificationDeclarationMessage set to true to notify the
+    // CastingPlayer/Commissioner that the commissioning session was cancelled by the Casting Client/Commissionee user. This will
+    // result in the Casting Client/Commissionee sending a CancelPasscode IdentificationDeclaration message to the CastingPlayer.
+    // shouldSendIdentificationDeclarationMessage is true when StopConnecting() is called by the Client.
     return this->StopConnecting(true);
 }
 
@@ -253,24 +253,13 @@ CHIP_ERROR CastingPlayer::StopConnecting(bool shouldSendIdentificationDeclaratio
     mTargetCastingPlayer.reset();
     CastingPlayerDiscovery::GetInstance()->ClearCastingPlayersInternal();
 
-    // shouldSendIdentificationDeclarationMessage is false only when StopConnecting() is called by the casting library.
     if (!shouldSendIdentificationDeclarationMessage)
     {
         ChipLogProgress(AppServer,
-                        "CastingPlayer::StopConnecting() shouldSendIdentificationDeclarationMessage: %s, User Directed "
-                        "Commissioning stopped by CastingPlayer/Commissioner user",
-                        shouldSendIdentificationDeclarationMessage ? "true" : "false");
-        // If the client has not provided the (Optional) ConnectionCallbacks mCommissionerDeclarationCallback, we can call the
-        // ConnectionCallbacks mOnConnectionComplete callback with CHIP_ERROR_CONNECTION_ABORTED, to alert the client that the
-        // commissioning session was cancelled by the CastingPlayer/Commissioner user. For example, in the scenario where user 1 is
-        // controlling the casting Client/Commissionee and user 2 is controlling the CastingPlayer/Commissioner.
-        if (!mClientProvidedCommissionerDeclarationCallback)
-        {
-            ChipLogProgress(AppServer,
-                            "CastingPlayer::StopConnecting() CommissionerDeclarationCallback not provided by client, calling "
-                            "mOnConnectionComplete with CHIP_ERROR_CONNECTION_ABORTED");
-            mOnCompleted(CHIP_ERROR_CONNECTION_ABORTED, nullptr);
-        }
+                        "CastingPlayer::StopConnecting() shouldSendIdentificationDeclarationMessage: %d, User Directed "
+                        "Commissioning aborted by the CastingPlayer/Commissioner user.",
+                        shouldSendIdentificationDeclarationMessage);
+        resetState(CHIP_ERROR_CONNECTION_ABORTED);
         return err;
     }
 

examples/tv-casting-app/tv-casting-common/core/CommissionerDeclarationHandler.cpp

@@ -57,15 +57,29 @@ void CommissionerDeclarationHandler::OnCommissionerDeclarationMessage(
         ChipLogProgress(AppServer,
                         "CommissionerDeclarationHandler::OnCommissionerDeclarationMessage(), Got CancelPasscode parameter, "
                         "CastingPlayer/Commissioner user has decided to exit the commissioning attempt. Connection aborted.");
-        // Since the user has decided to exit the commissioning process on the CastingPlayer/Commissioner, we cancel the ongoing
-        // connection attempt without notifying the CastingPlayer/Commissioner.
-        if (auto sharedPtr = mTargetCastingPlayer.lock())
+        // Since the CastingPlayer/Commissioner user has decided to exit the commissioning process, we cancel the ongoing
+        // connection attempt without notifying the CastingPlayer/Commissioner. Therefore the
+        // shouldSendIdentificationDeclarationMessage flag in the internal StopConnecting() API call is set to false. The
+        // CastingPlayer/Commissioner user and the Casting Client/Commissionee user are not necessarily the same user. For example,
+        // in an enviroment with multiple CastingPlayer/Commissioner TVs, one user 1 might be controlling the Client/Commissionee
+        // and user 2 might be controlling the CastingPlayer/Commissioner TV.
+        CastingPlayer * targetCastingPlayer = CastingPlayer::GetTargetCastingPlayer();
+        // Avoid crashing if we recieve this CommissionerDeclaration message when targetCastingPlayer is nullptr.
+        if (targetCastingPlayer == nullptr)
         {
-            CHIP_ERROR err = sharedPtr->StopConnecting(false);
+            ChipLogError(AppServer,
+                         "CommissionerDeclarationHandler::OnCommissionerDeclarationMessage() targetCastingPlayer is nullptr");
+        }
+        else
+        {
+            CHIP_ERROR err = targetCastingPlayer->StopConnecting(false);
             if (err != CHIP_NO_ERROR)
             {
-                ChipLogError(AppServer,
-                             "CommissionerDeclarationHandler::OnCommissionerDeclarationMessage() Failed to StopConnecting");
+                ChipLogError(
+                    AppServer,
+                    "CommissionerDeclarationHandler::OnCommissionerDeclarationMessage() StopConnecting() failed due to error: "
+                    "%" CHIP_ERROR_FORMAT,
+                    err.Format());
             }
         }
     }
@@ -82,14 +96,13 @@ void CommissionerDeclarationHandler::OnCommissionerDeclarationMessage(
 }
 
 void CommissionerDeclarationHandler::SetCommissionerDeclarationCallback(
-    matter::casting::core::CommissionerDeclarationCallback callback, memory::Weak<CastingPlayer> castingPlayer)
+    matter::casting::core::CommissionerDeclarationCallback callback)
 {
     ChipLogProgress(AppServer, "CommissionerDeclarationHandler::SetCommissionerDeclarationCallback()");
     if (callback != nullptr)
     {
         mCmmissionerDeclarationCallback_ = std::move(callback);
     }
-    mTargetCastingPlayer = castingPlayer;
 }
 
 } // namespace core

examples/tv-casting-app/tv-casting-common/core/CommissionerDeclarationHandler.h

@@ -48,7 +48,7 @@ class CommissionerDeclarationHandler : public chip::Protocols::UserDirectedCommi
     /**
      * @brief OnCommissionerDeclarationMessage() will call the CommissionerDeclarationCallback set by this function.
      */
-    void SetCommissionerDeclarationCallback(CommissionerDeclarationCallback callback, memory::Weak<CastingPlayer> castingPlayer);
+    void SetCommissionerDeclarationCallback(CommissionerDeclarationCallback callback);
 
     /**
      * @brief returns true if the CommissionerDeclarationHandler sigleton has a CommissionerDeclarationCallback set, false
@@ -59,7 +59,6 @@ class CommissionerDeclarationHandler : public chip::Protocols::UserDirectedCommi
 private:
     static CommissionerDeclarationHandler * sCommissionerDeclarationHandler_;
     CommissionerDeclarationCallback mCmmissionerDeclarationCallback_;
-    memory::Weak<CastingPlayer> mTargetCastingPlayer;
 
     CommissionerDeclarationHandler() {}
     ~CommissionerDeclarationHandler() {}