Revert "Enable PageInfoV2Desktop by default"

This reverts commit 842626c. Reason for revert: [sheriff] suspected to cause multiple failures in SafetyTipPageInfoBubbleViewBrowserTests on linux-ubsan-vptr: https://ci.chromium.org/p/chromium/builders/ci/linux-ubsan-vptr/6970 Sample failure: [ RUN ] All/SafetyTipPageInfoBubbleViewBrowserTest.BubbleWaitsForVisible/1 [...] ../../chrome/browser/ui/views/page_info/safety_tip_page_info_bubble_view_browsertest.cc:390:23: runtime error: downcast of address 0x180601885e00 which does not point to an object of type 'PageInfoBubbleView' 0x180601885e00: note: object is of type 'PageInfoNewBubbleView' 00 00 00 00 18 70 11 10 50 56 00 00 80 68 50 01 06 18 00 00 b8 00 00 00 30 00 00 00 00 00 00 00 ^~~~~~~~~~~~~~~~~~~~~~~ vptr for 'PageInfoNewBubbleView' #0 0x564ffa77d440 in SafetyTipPageInfoBubbleViewBrowserTest::CheckPageInfoShowsSafetyTipInfo(Browser*, security_state::SafetyTipStatus, GURL const&) chrome/browser/ui/views/page_info/safety_tip_page_info_bubble_view_browsertest.cc:390:23 #1 0x564ffa788397 in SafetyTipPageInfoBubbleViewBrowserTest_BubbleWaitsForVisible_Test::RunTestOnMainThread() chrome/browser/ui/views/page_info/safety_tip_page_info_bubble_view_browsertest.cc:837:3 #2 0x565002ae1026 in content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() content/public/test/browser_test_base.cc:868:7 #3 0x565001aae15a in Run base/callback.h:142:12 #4 0x565001aae15a in ChromeBrowserMainParts::PreMainMessageLoopRunImpl() chrome/browser/chrome_browser_main.cc:1745:38 #5 0x565001aaba50 in ChromeBrowserMainParts::PreMainMessageLoopRun() chrome/browser/chrome_browser_main.cc:1083:18 #6 0x564ffd8e2f00 in content::BrowserMainLoop::PreMainMessageLoopRun() content/browser/browser_main_loop.cc:957:28 #7 0x564ffe307f82 in Run base/callback.h:142:12 #8 0x564ffe307f82 in content::StartupTaskRunner::RunAllTasksNow() content/browser/startup_task_runner.cc:41:29 #9 0x564ffd8e2368 in content::BrowserMainLoop::CreateStartupTasks() content/browser/browser_main_loop.cc:865:25 #10 0x564ffd8e889c in content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) content/browser/browser_main_runner_impl.cc:131:15 #11 0x564ffd8dec1e in content::BrowserMain(content::MainFunctionParams const&) content/browser/browser_main.cc:45:32 #12 0x564fff2a06e8 in content::RunBrowserProcessMain(content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner_impl.cc:641:10 #13 0x564fff2a2e90 in content::ContentMainRunnerImpl::RunBrowser(content::MainFunctionParams&, bool) content/app/content_main_runner_impl.cc:1137:10 #14 0x564fff2a1f18 in content::ContentMainRunnerImpl::Run(bool) content_main_runner_impl.cc #15 0x564fff29dde0 in content::RunContentProcess(content::ContentMainParams const&, content::ContentMainRunner*) content_main.cc #16 0x564fff29e7dd in content::ContentMain(content::ContentMainParams const&) content_main.cc #17 0x565002adfcff in content::BrowserTestBase::SetUp() content/public/test/browser_test_base.cc:712:3 #18 0x5650018271c4 in InProcessBrowserTest::SetUp() chrome/test/base/in_process_browser_test.cc:401:20 #19 0x564ffa79d759 in SafetyTipPageInfoBubbleViewBrowserTest::SetUp() chrome/browser/ui/views/page_info/safety_tip_page_info_bubble_view_browsertest.cc:306:27 #20 0x564ffb8f8c7d in testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc:2701:3 #21 0x564ffb8fa600 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2885:11 #22 0x564ffb8fc1bb in testing::TestSuite::Run() third_party/googletest/src/googletest/src/gtest.cc:3044:30 #23 0x564ffb9115ca in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5903:44 #24 0x564ffb90fd8c in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:5470:10 #25 0x565001a12e28 in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2492:46 #26 0x565001a12e28 in base::TestSuite::Run() base/test/test_suite.cc:445:16 #27 0x5650017d0bac in ChromeTestSuiteRunner::RunTestSuiteInternal(ChromeTestSuite*) chrome/test/base/chrome_test_launcher.cc:88:22 #28 0x5650017d0c1b in ChromeTestSuiteRunner::RunTestSuite(int, char**) chrome_test_launcher.cc #29 0x565002b8529b in content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) content/public/test/test_launcher.cc:409:31 #30 0x5650017d11b1 in LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) chrome/test/base/chrome_test_launcher.cc:284:10 #31 0x5650017c9dcc in main chrome/test/base/browser_tests_main.cc:61:10 #32 0x7fc242b01bf6 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21bf6) #33 0x564ff76da7e9 in _start (/b/s/w/ir/out/Release/browser_tests+0xe8af7e9) SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../chrome/browser/ui/views/page_info/safety_tip_page_info_bubble_view_browsertest.cc:390:23 in Original change's description: > Enable PageInfoV2Desktop by default > > Enable flag and remove testing config. > > Bug: 1188101 > Change-Id: Ifd839436f3d4c0127d15b449bbca84855ccd858f > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3217207 > Reviewed-by: Balazs Engedy <engedy@chromium.org> > Reviewed-by: Christian Dullweber <dullweber@chromium.org> > Reviewed-by: Theodore Olsauskas-Warren <sauski@google.com> > Reviewed-by: Xinghui Lu <xinghuilu@chromium.org> > Commit-Queue: Olesia Marukhno <olesiamarukhno@google.com> > Cr-Commit-Position: refs/heads/main@{#931972} Bug: 1188101 Change-Id: Ic51b9fdc730b7046e24b4d394248218b86eef09d No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3226638 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Owners-Override: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/heads/main@{#932067}
blueboxd · Oct 15, 2021 · 30f6e0b · 30f6e0b
1 parent 96acdfb
commit 30f6e0b
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 31 deletions.
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
@@ -47,9 +47,7 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/location_bar/location_icon_view.h"
-#include "chrome/browser/ui/views/page_info/page_info_bubble_view.h"
 #include "chrome/browser/ui/views/page_info/page_info_bubble_view_base.h"
-#include "chrome/browser/ui/views/page_info/page_info_view_factory.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/pref_names.h"
@@ -58,7 +56,6 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/google/core/common/google_util.h"
 #include "components/omnibox/browser/omnibox_prefs.h"
-#include "components/page_info/features.h"
 #include "components/permissions/permission_util.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/core/common/policy_types.h"
@@ -2013,17 +2010,6 @@ class SafeBrowsingBlockingPageDelayedWarningBrowserTest
         ->AddDangerousUrl(url, threat_type);
   }
 
-  std::u16string GetSecuritySummaryTextFromPageInfo() {
-    auto* page_info = PageInfoBubbleView::GetPageInfoBubbleForTesting();
-    if (base::FeatureList::IsEnabled(page_info::kPageInfoV2Desktop)) {
-      auto* summary_label = page_info->GetViewByID(
-          PageInfoViewFactory::VIEW_ID_PAGE_INFO_SECURITY_SUMMARY_LABEL);
-      return static_cast<views::StyledLabel*>(summary_label)->GetText();
-    }
-
-    return page_info->GetWindowTitle();
-  }
-
  protected:
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   // Installs an extension and returns its ID.
@@ -2674,7 +2660,7 @@ IN_PROC_BROWSER_TEST_P(
   auto* page_info = OpenPageInfo(browser());
   ASSERT_TRUE(page_info);
   EXPECT_EQ(
-      GetSecuritySummaryTextFromPageInfo(),
+      page_info->GetWindowTitle(),
       l10n_util::GetStringUTF16(IDS_PAGE_INFO_SAFETY_TIP_BAD_REPUTATION_TITLE));
 
   // When the Safety Tip is showing, the security level should be downgraded, as
@@ -2781,7 +2767,7 @@ IN_PROC_BROWSER_TEST_P(
             PageInfoBubbleViewBase::GetShownBubbleType());
   auto* page_info = OpenPageInfo(browser());
   ASSERT_TRUE(page_info);
-  EXPECT_EQ(GetSecuritySummaryTextFromPageInfo(),
+  EXPECT_EQ(page_info->GetWindowTitle(),
             l10n_util::GetStringFUTF16(IDS_PAGE_INFO_SAFETY_TIP_LOOKALIKE_TITLE,
                                        u"google.com"));
 }

diff --git a/chrome/browser/ui/hats/trust_safety_sentiment_service_browsertest.cc b/chrome/browser/ui/hats/trust_safety_sentiment_service_browsertest.cc
@@ -10,12 +10,10 @@
 #include "chrome/browser/ui/hats/mock_hats_service.h"
 #include "chrome/browser/ui/page_info/page_info_dialog.h"
 #include "chrome/browser/ui/views/page_info/page_info_bubble_view.h"
-#include "chrome/browser/ui/views/page_info/page_info_new_bubble_view.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/webui_url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
-#include "components/page_info/features.h"
 #include "content/public/test/browser_test.h"
 
 using ::testing::_;
@@ -48,21 +46,16 @@ class TrustSafetySentimentServiceBrowserTest : public InProcessBrowserTest {
     base::RunLoop().RunUntilIdle();
   }
 
-  PageInfo* GetPresenter() {
-    auto* bubble = PageInfoBubbleView::GetPageInfoBubbleForTesting();
-    return base::FeatureList::IsEnabled(page_info::kPageInfoV2Desktop)
-               ? static_cast<PageInfoNewBubbleView*>(bubble)->presenter_.get()
-               : static_cast<PageInfoBubbleView*>(bubble)->presenter_.get();
-  }
-
   void ChangePermission() {
     PageInfo::PermissionInfo permission;
     permission.type = ContentSettingsType::NOTIFICATIONS;
     permission.setting = ContentSetting::CONTENT_SETTING_BLOCK;
     permission.default_setting = ContentSetting::CONTENT_SETTING_ASK;
+    permission.source = content_settings::SettingSource::SETTING_SOURCE_USER;
 
-    GetPresenter()->OnSitePermissionChanged(permission.type, permission.setting,
-                                            permission.is_one_time);
+    static_cast<PageInfoBubbleView*>(
+        PageInfoBubbleView::GetPageInfoBubbleForTesting())
+        ->OnPermissionChanged(permission);
   }
 
   void OpenEnoughNewTabs() {

diff --git a/chrome/browser/ui/views/page_info/page_info_new_bubble_view.h b/chrome/browser/ui/views/page_info/page_info_new_bubble_view.h
@@ -48,7 +48,6 @@ class PageInfoNewBubbleView : public PageInfoBubbleViewBase,
   friend class PageInfoBubbleViewBrowserTest;
   friend class PageInfoBubbleViewDialogBrowserTest;
   friend class test::PageInfoBubbleViewTestApi;
-  friend class TrustSafetySentimentServiceBrowserTest;
 
   // PageInfoBubbleViewBase:
   gfx::Size CalculatePreferredSize() const override;

diff --git a/chrome/browser/ui/views/page_info/safety_tip_page_info_bubble_view_browsertest.cc b/chrome/browser/ui/views/page_info/safety_tip_page_info_bubble_view_browsertest.cc
@@ -365,6 +365,13 @@ class SafetyTipPageInfoBubbleViewBrowserTest
     return AreLookalikeWarningsEnabled() ? IsUIShowing() : !IsUIShowing();
   }
 
+  const std::u16string GetPageInfoBubbleViewSummaryText() {
+    auto* label =
+        PageInfoBubbleView::GetPageInfoBubbleForTesting()->GetViewByID(
+            PageInfoViewFactory::VIEW_ID_PAGE_INFO_SECURITY_SUMMARY_LABEL);
+    return static_cast<views::StyledLabel*>(label)->GetText();
+  }
+
   std::u16string GetSafetyTipSummaryText() {
     auto* page_info = PageInfoBubbleView::GetPageInfoBubbleForTesting();
     if (base::FeatureList::IsEnabled(page_info::kPageInfoV2Desktop)) {
@@ -703,9 +710,8 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
   TriggerWarningFromBlocklist(browser(), kNavigatedUrl,
                               WindowOpenDisposition::CURRENT_TAB);
   ASSERT_NO_FATAL_FAILURE(CheckNoButtons());
-  auto* page_info = PageInfoBubbleView::GetPageInfoBubbleForTesting();
   EXPECT_EQ(
-      page_info->GetWindowTitle(),
+      GetSafetyTipSummaryText(),
       l10n_util::GetStringUTF16(IDS_PAGE_INFO_SAFETY_TIP_BAD_REPUTATION_TITLE));
 }
 

diff --git a/components/page_info/features.cc b/components/page_info/features.cc
@@ -18,7 +18,7 @@ const base::Feature kPageInfoStoreInfo{"PageInfoStoreInfo",
 
 #if !defined(OS_ANDROID)
 const base::Feature kPageInfoV2Desktop{"PageInfoV2Desktop",
-                                       base::FEATURE_ENABLED_BY_DEFAULT};
+                                       base::FEATURE_DISABLED_BY_DEFAULT};
 #endif
 
 const base::Feature kPageInfoAboutThisSite{"PageInfoAboutThisSite",

diff --git a/testing/variations/fieldtrial_testing_config.json b/testing/variations/fieldtrial_testing_config.json
@@ -5769,6 +5769,24 @@
             ]
         }
     ],
+    "PageInfoV2Desktop": [
+        {
+            "platforms": [
+                "chromeos",
+                "linux",
+                "mac",
+                "windows"
+            ],
+            "experiments": [
+                {
+                    "name": "Enabled",
+                    "enable_features": [
+                        "PageInfoV2Desktop"
+                    ]
+                }
+            ]
+        }
+    ],
     "PaintPreviewShowOnStartup": [
         {
             "platforms": [