Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Brave shields #1

Merged
merged 24 commits into from
Dec 17, 2017
Merged

Add Brave shields #1

merged 24 commits into from
Dec 17, 2017

Conversation

bbondy
Copy link
Member

@bbondy bbondy commented Nov 15, 2017

No description provided.

@bbondy bbondy force-pushed the shields branch 2 times, most recently from a757f83 to 1abaf8d Compare November 23, 2017 22:24
mariospr added a commit that referenced this pull request Mar 23, 2022
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.
@pes10k pes10k mentioned this pull request Apr 27, 2022
25 tasks
@fmarier fmarier mentioned this pull request Nov 7, 2022
25 tasks
@sujitacharya2005 sujitacharya2005 mentioned this pull request Jun 12, 2023
25 tasks
vadimstruts added a commit that referenced this pull request May 15, 2024
Signed-off-by: Vadym Struts <vstruts@brave.com>
vadimstruts added a commit that referenced this pull request Jun 4, 2024
Signed-off-by: Vadym Struts <vstruts@brave.com>
vadimstruts added a commit that referenced this pull request Jun 7, 2024
Signed-off-by: Vadym Struts <vstruts@brave.com>
vadimstruts added a commit that referenced this pull request Jun 17, 2024
Signed-off-by: Vadym Struts <vstruts@brave.com>
vadimstruts added a commit that referenced this pull request Jun 28, 2024
Signed-off-by: Vadym Struts <vstruts@brave.com>
vadimstruts added a commit that referenced this pull request Jul 3, 2024
Signed-off-by: Vadym Struts <vstruts@brave.com>
vadimstruts added a commit that referenced this pull request Jul 8, 2024
Signed-off-by: Vadym Struts <vstruts@brave.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants