Add Brave shields #1

bbondy · 2017-11-15T17:57:18Z

No description provided.

This fixes several upstream browser tests crashing on DCHECK-enabled builds because of calls to dom_distiller::RunIsolatedJavaScript() being run without the ID being previously set, with erros like this: [497557:497557:1221/135319.265636:FATAL:distiller_javascript_utils.cc(41)] Check failed: distiller_javascript_world_id != invalid_world_id. #0 0x7fd8cc5b7459 base::debug::CollectStackTrace() #1 0x7fd8cc4b12d3 base::debug::StackTrace::StackTrace() #2 0x7fd8cc4d3a34 logging::LogMessage::~LogMessage() #3 0x7fd8cc4d44ae logging::LogMessage::~LogMessage() #4 0x56550ee1de70 dom_distiller::RunIsolatedJavaScript() #5 0x56550fffc990 brave_ads::AdsTabHelper::RunIsolatedJavaScript() #6 0x7fd8c6332b3d content::WebContentsImpl::WebContentsObserverList::NotifyObservers<>() #7 0x7fd8c63531a9 content::WebContentsImpl::DocumentOnLoadCompleted() [...] #56 0x56550ccba27a _start Task trace: #0 0x7fd8c6c6ebee IPC::(anonymous namespace)::ChannelAssociatedGroupController::Accept() #1 0x7fd8cbce69df mojo::SimpleWatcher::Context::Notify() Crash keys: "ui_scheduler_async_stack" = "0x7FD8C6C6EBEE 0x7FD8CBCE69DF" "io_scheduler_async_stack" = "0x7FD8CBCE69DF 0x0" [1/1] TabStripBrowsertest.ShiftGroupLeft_OtherGroup (CRASHED) This is necessary or all browser tests will crash when running on CI.

BraveMainDelegate::BasicStartupComplete() is where several features are enabled and disabled for Brave via command line switches, but that method is not being called when running usptream tests, which use ChromeMainDelegate instead. Because of this, all browser tests crash on DCHECK-enabled builds with a backtrace like the following one: [497557:497557:1221/135319.265636:FATAL:distiller_javascript_utils.cc(41)] Check failed: distiller_javascript_world_id != invalid_world_id. #0 0x7fd8cc5b7459 base::debug::CollectStackTrace() #1 0x7fd8cc4b12d3 base::debug::StackTrace::StackTrace() #2 0x7fd8cc4d3a34 logging::LogMessage::~LogMessage() #3 0x7fd8cc4d44ae logging::LogMessage::~LogMessage() #4 0x56550ee1de70 dom_distiller::RunIsolatedJavaScript() #5 0x56550fffc990 brave_ads::AdsTabHelper::RunIsolatedJavaScript() #6 0x7fd8c6332b3d content::WebContentsImpl::WebContentsObserverList::NotifyObservers<>() #7 0x7fd8c63531a9 content::WebContentsImpl::DocumentOnLoadCompleted() [...] #56 0x56550ccba27a _start Task trace: #0 0x7fd8c6c6ebee IPC::(anonymous namespace)::ChannelAssociatedGroupController::Accept() #1 0x7fd8cbce69df mojo::SimpleWatcher::Context::Notify() Crash keys: "ui_scheduler_async_stack" = "0x7FD8C6C6EBEE 0x7FD8CBCE69DF" "io_scheduler_async_stack" = "0x7FD8CBCE69DF 0x0" [1/1] TabStripBrowsertest.ShiftGroupLeft_OtherGroup (CRASHED) This is because the DOM Distiller feature is not enabled as expected (for Brave) on startup, and when AdsTabHelper::RunIsolatedJavaScript() (called via the WebContentsObserver machinery) calls dom_distiller::RunIsolatedJavaScript() it will fail the DCHECK that makes sure the JS World ID is already set. To fix this, we move the code of BraveMainDelegate::BasicStartupComplete() to ChromeMainDelegate::BasicStartupComplete() via a chromium_src override, so that we make sure that exactly the same initialization done for Brave and Brave-related browser tests also applies when running upstrem browser tests. This does not only fix that crash, but also makes sure that upstream browser tests are run with the same features as Brave, which is the right thing to do.

Chromium change: https://source.chromium.org/chromium/chromium/src/+/072b6bf1b7db7f8b8cf2e2e3127a98a6b859d7bb commit 072b6bf1b7db7f8b8cf2e2e3127a98a6b859d7bb Author: Devlin Cronin <rdevlin.cronin@chromium.org> Date: Tue Feb 8 19:40:21 2022 +0000 [Extensions UI] Simplify the flow of programmatically showing popups Extensions can programmatically open their toolbar action popups. Before this CL, the flow for this was: API Call -> ExtensionsToolbarContainer::ShowToolbarActionPopupForAPICall() -> ExtensionActionViewController::ExecuteAction() (#1) -> ExtensionActionViewController::ExecuteAction() (#2) -> TriggerPopup() This has the following issues: - ExecuteAction() is also used for executing other types of behaviors (like triggering an event listener), not just for opening popups. - ExecuteAction() may grant the extension tab permissions if it was indicated that the call was through a user gesture Both of these aren't *technically* issues, since we theoretically validate that the extension will open a popup (and not fire a listener) and never trigger the call as a user gesture. But, it's a tenuous balance. Refactor this to introduce a public TriggerPopupForAPI() method on ExtensionActionViewController that ExtensionsToolbarContainer can call directly. This clears up the issues above by making the contract and expectations clear, and ensures that there's no way we'd ever grant tab permissions or fire an event for a programmatic API call. As a bonus, we simplify the interface by removing one of the ExecuteAction() overloads and the need to specify `by_user` in the call, instead just having ExecuteUserAction(). This will also make it easier to plumb through a callback for programmatic API calls when we need to wait for the popup to open. Bug: 1245093