[AndroidSms] Fix issue with background notifications after migration.

This CL fixes issue with background notifications not working after PWA migration. This was caused due to StreamingConnectionEstablisher not clearing state in TearDown. Also fixed related issue with stopping service worker in TearDown. Bug: 939581 Change-Id: Icfda555117fac726b39f34b8a601dc447c9d6ef5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1510655 Reviewed-by: Jeremy Klein <[email protected]> Commit-Queue: Azeem Arshad <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#639129}(cherry picked from commit a9aab3a9d68f69e0482bff47a00dfaf07aaeeaee) Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1518849 Reviewed-by: Azeem Arshad <[email protected]> Cr-Commit-Position: refs/branch-heads/3729@{#68} Cr-Branched-From: d4a8972-refs/heads/master@{#638880}
nwjs · Mar 12, 2019 · 911bfc3 · 911bfc3
1 parent 1606ed6
commit 911bfc3
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 2 deletions.
diff --git a/chrome/browser/chromeos/android_sms/connection_manager.cc b/chrome/browser/chromeos/android_sms/connection_manager.cc
@@ -138,6 +138,8 @@ void ConnectionManager::UpdateConnectionStatus() {
     connection_establisher_->TearDownConnection(
         *enabled_pwa_url_, GetCurrentServiceWorkerContext());
     GetCurrentServiceWorkerContext()->RemoveObserver(this);
+    active_version_id_.reset();
+    prev_active_version_id_.reset();
   }
 
   enabled_pwa_url_ = updated_pwa_url;

diff --git a/chrome/browser/chromeos/android_sms/connection_manager_unittest.cc b/chrome/browser/chromeos/android_sms/connection_manager_unittest.cc
@@ -306,6 +306,11 @@ TEST_F(ConnectionManagerTest, FeatureStateChange) {
   // Verify that enabling feature establishes connection again.
   SetPwaState(PwaState::kEnabledWithNewUrl);
   VerifyEstablishConnectionCalls(3u /* expected_count */);
+
+  // Verify that connection is established if the version id changes.
+  fake_new_service_worker_context()->NotifyObserversOnNoControllees(
+      kDummyVersionId2, GetAndroidMessagesURL());
+  VerifyEstablishConnectionCalls(4u /* expected_count */);
 }
 
 TEST_F(ConnectionManagerTest, AppUrlMigration) {

diff --git a/chrome/browser/chromeos/android_sms/streaming_connection_establisher.cc b/chrome/browser/chromeos/android_sms/streaming_connection_establisher.cc
@@ -45,7 +45,8 @@ void StreamingConnectionEstablisher::EstablishConnection(
 void StreamingConnectionEstablisher::TearDownConnection(
     const GURL& url,
     content::ServiceWorkerContext* service_worker_context) {
-  service_worker_context->StopAllServiceWorkersForOrigin(url);
+  service_worker_context->StopAllServiceWorkersForOrigin(url.GetOrigin());
+  is_connected_ = false;
 }
 
 void StreamingConnectionEstablisher::SendStartStreamingMessageIfNotConnected(

diff --git a/chrome/browser/chromeos/android_sms/streaming_connection_establisher_unittest.cc b/chrome/browser/chromeos/android_sms/streaming_connection_establisher_unittest.cc
@@ -117,7 +117,19 @@ TEST_F(StreamingConnectionEstablisherTest, TearDownConnection) {
   const auto& stop_calls =
       fake_service_worker_context.stop_all_service_workers_for_origin_calls();
   ASSERT_EQ(1u, stop_calls.size());
-  EXPECT_EQ(GetAndroidMessagesURL(), stop_calls[0]);
+  EXPECT_EQ(GetAndroidMessagesURL().GetOrigin(), stop_calls[0]);
+
+  // Verify that subsequent message message dispatch calls succeed.
+  auto& message_dispatch_calls =
+      fake_service_worker_context
+          .start_service_worker_and_dispatch_long_running_message_calls();
+
+  connection_establisher.EstablishConnection(
+      GetAndroidMessagesURL(),
+      ConnectionEstablisher::ConnectionMode::kStartConnection,
+      &fake_service_worker_context);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(1u, message_dispatch_calls.size());
 }
 
 }  // namespace android_sms