Fixed crash when toggling playlist panel while it's playing (uplift to 1.78.x)

browser/playlist/test/BUILD.gn

@@ -41,7 +41,10 @@ source_set("browser_tests") {
   if (is_android) {
     deps += [ "//chrome/test:test_support_ui_android" ]
   } else {
-    deps += [ "//chrome/test:test_support_ui" ]
+    deps += [
+      "//chrome/browser/ui/browser_window",
+      "//chrome/test:test_support_ui",
+    ]
   }
 }
 

browser/playlist/test/playlist_browsertest.cc

@@ -11,6 +11,7 @@
 #include "base/run_loop.h"
 #include "base/test/bind.h"
 #include "base/test/mock_callback.h"
+#include "base/test/run_until.h"
 #include "base/test/scoped_feature_list.h"
 #include "brave/browser/playlist/playlist_service_factory.h"
 #include "brave/browser/ui/brave_browser.h"
@@ -32,7 +33,10 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_command_controller.h"
+#include "chrome/browser/ui/browser_window/public/browser_window_features.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/side_panel/side_panel_entry_id.h"
+#include "chrome/browser/ui/views/side_panel/side_panel_ui.h"
 #include "chrome/browser/ui/views/tabs/tab.h"
 #include "chrome/test/base/platform_browser_test.h"
 #include "content/public/test/browser_test.h"
@@ -168,6 +172,42 @@ class PlaylistBrowserTest : public PlatformBrowserTest {
   content::ContentMockCertVerifier mock_cert_verifier_;
 };
 
+// Check toggling playlist panel while playing doesn't make crash.
+IN_PROC_BROWSER_TEST_F(PlaylistBrowserTest, PanelToggleTestWhilePlaying) {
+  auto* panel_ui = browser()->GetFeatures().side_panel_ui();
+
+  // Open playlist panel.
+  panel_ui->Show(SidePanelEntryId::kPlaylist);
+  WaitUntil(base::BindLambdaForTesting(
+      [&]() { return panel_ui->IsSidePanelShowing(); }));
+  ASSERT_TRUE(
+      base::test::RunUntil([&]() { return panel_ui->IsSidePanelShowing(); }));
+
+  auto* coordinator = PlaylistSidePanelCoordinator::FromBrowser(browser());
+  ASSERT_TRUE(coordinator);
+  coordinator->is_audible_for_testing_ = true;
+
+  // Close playlist panel check cached instances are still live.
+  panel_ui->Close();
+  EXPECT_TRUE(coordinator->contents_wrapper_);
+  ASSERT_TRUE(
+      base::test::RunUntil([&]() { return !panel_ui->IsSidePanelShowing(); }));
+
+  // Re-open playlist panel.
+  panel_ui->Show(SidePanelEntryId::kPlaylist);
+  ASSERT_TRUE(
+      base::test::RunUntil([&]() { return panel_ui->IsSidePanelShowing(); }));
+
+  // Not audible. Check cached webview/contents are destroyed.
+  coordinator->is_audible_for_testing_ = false;
+
+  // Close playlist panel. Check cached instances are all freed.
+  panel_ui->Close();
+  EXPECT_FALSE(coordinator->contents_wrapper_);
+  ASSERT_TRUE(
+      base::test::RunUntil([&]() { return !panel_ui->IsSidePanelShowing(); }));
+}
+
 IN_PROC_BROWSER_TEST_F(PlaylistBrowserTest, AddItemsToList) {
   ASSERT_TRUE(content::NavigateToURL(GetActiveWebContents(),
                                      GetURL("/playlist/site_with_video.html")));

browser/ui/views/side_panel/playlist/playlist_side_panel_coordinator.cc

@@ -7,6 +7,7 @@
 
 #include <memory>
 
+#include "base/check_is_test.h"
 #include "base/functional/callback.h"
 #include "brave/browser/ui/brave_browser.h"
 #include "brave/browser/ui/sidebar/sidebar_controller.h"
@@ -22,6 +23,8 @@
 #include "chrome/browser/ui/views/side_panel/side_panel_util.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/grit/brave_components_strings.h"
+#include "content/public/browser/visibility.h"
+#include "content/public/browser/web_contents.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/models/image_model.h"
 #include "ui/views/vector_icons.h"
@@ -88,6 +91,16 @@ std::unique_ptr<views::View> PlaylistSidePanelCoordinator::CreateWebView(
 
     Proxy::CreateForWebContents(contents_wrapper_->web_contents(),
                                 weak_ptr_factory_.GetWeakPtr());
+  } else {
+    // Set visible to avoid CHECK(page_node->IsVisible()) failure in
+    // SidePanelLoadingVoter::MarkAsSidePanel(). When SidePanelWebView is
+    // created below, upstream marks this content and has assumption that it's
+    // visible if it has loaded url. When playlist panel is closed while
+    // playing, we cache |contents_wrapper_| to make it continue to play after
+    // closing panel. So, it has loaded url already. Should be visible before
+    // creating SidePanelWebView with it to avoid above CHECK failure.
+    contents_wrapper_->web_contents()->UpdateWebContentsVisibility(
+        content::Visibility::VISIBLE);
   }
 
   auto web_view = std::make_unique<PlaylistSidePanelWebView>(
@@ -118,6 +131,12 @@ void PlaylistSidePanelCoordinator::OnViewIsDeleting(views::View* view) {
 
 void PlaylistSidePanelCoordinator::DestroyWebContentsIfNeeded() {
   DCHECK(contents_wrapper_);
+
+  if (is_audible_for_testing_) {
+    CHECK_IS_TEST();
+    return;
+  }
+
   if (!contents_wrapper_->web_contents()->IsCurrentlyAudible()) {
     contents_wrapper_.reset();
   }

browser/ui/views/side_panel/playlist/playlist_side_panel_coordinator.h

@@ -9,6 +9,7 @@
 #include <memory>
 #include <string>
 
+#include "base/gtest_prod_util.h"
 #include "base/scoped_observation.h"
 #include "brave/browser/ui/views/side_panel/playlist/playlist_contents_wrapper.h"
 #include "brave/browser/ui/views/side_panel/playlist/playlist_side_panel_web_view.h"
@@ -18,6 +19,7 @@
 
 namespace playlist {
 class PlaylistUI;
+FORWARD_DECLARE_TEST(PlaylistBrowserTest, PanelToggleTestWhilePlaying);
 }  // namespace playlist
 
 class Browser;
@@ -71,13 +73,16 @@ class PlaylistSidePanelCoordinator
 
  private:
   friend class BrowserUserData<PlaylistSidePanelCoordinator>;
+  FRIEND_TEST_ALL_PREFIXES(playlist::PlaylistBrowserTest,
+                           PanelToggleTestWhilePlaying);
 
   void DestroyWebContentsIfNeeded();
 
   std::unique_ptr<views::View> CreateWebView(SidePanelEntryScope& scope);
 
   raw_ptr<Browser> browser_ = nullptr;
 
+  bool is_audible_for_testing_ = false;
   std::unique_ptr<PlaylistContentsWrapper> contents_wrapper_;
 
   base::WeakPtr<PlaylistSidePanelWebView> side_panel_web_view_;