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

[ads] Follow up to #37390: Failing to Initialize ads due to corrupted confirmations.json #23083

Merged
merged 2 commits into from
Apr 15, 2024

Conversation

tmancey
Copy link
Collaborator

@tmancey tmancey commented Apr 15, 2024

Resolves brave/brave-browser#37589

Submitter Checklist:

  • I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally:
    • npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
    • npm run presubmit wiki, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

@tmancey tmancey self-assigned this Apr 15, 2024
@tmancey tmancey requested a review from a team as a code owner April 15, 2024 17:16
@tmancey
Copy link
Collaborator Author

tmancey commented Apr 15, 2024

[88221:259:0415/152416.550505:INFO:legacy_confirmation_migration.cc(64)] Confirmation state is corrupted, resetting to default values
[88221:259:0415/152416.551736:VERBOSE1:legacy_confirmation_migration.cc(78)] Migrating confirmation state
[88221:259:0415/152416.560425:VERBOSE3:confirmation_state_manager.cc(40)] Loading confirmation state
[88221:259:0415/152416.560876:VERBOSE3:confirmation_state_manager.cc(66)] Successfully loaded confirmation state
[88221:259:0415/152416.560934:VERBOSE1:ads_impl.cc(412)] Successfully initialized ads
[88221:259:0415/152416.562209:VERBOSE1:account.cc(65)] Successfully initialized wallet

Copy link
Contributor

[puLL-Merge] - brave/brave-core@23083

Here is my review:

Description

This PR removes the legacy rewards migration code from the brave-core ads component. The rewards migration was responsible for migrating legacy Brave Rewards state to the new Ads database. Now that the migration is complete, this code can be removed.

Changes

Changes

  • components/brave_ads/core/internal/BUILD.gn: Removes all build targets related to the legacy rewards migration
  • components/brave_ads/core/internal/account/transactions/transactions_database_table.cc: Removes reference to kMigrationUnreconciledTransactionId constant
  • components/brave_ads/core/internal/ads_impl.cc, components/brave_ads/core/internal/ads_impl.h: Removes MigrateRewardsState function and callback
  • components/brave_ads/core/internal/deprecated/confirmations/confirmation_state_manager.cc: Simplifies loading of confirmation tokens and payment tokens, removes migration failure dumps
  • components/brave_ads/core/internal/legacy_migration/confirmations/legacy_confirmation_migration.cc: Updates migration version for confirmation state
  • Deletes the entire brave/components/brave_ads/core/internal/legacy_migration/rewards/ directory and all its contents, which contained the legacy rewards migration code
  • components/brave_ads/core/public/prefs/pref_names.h: Updates confirmation state migration version
  • components/brave_ads/core/test/BUILD.gn: Removes unit test for legacy rewards migration

Security Hotspots

None identified. This change removes legacy code and does not introduce any new attack surface.

@tmancey
Copy link
Collaborator Author

tmancey commented Apr 15, 2024

Followup to brave/brave-browser#37390

@tmancey tmancey merged commit 1253872 into master Apr 15, 2024
19 checks passed
@tmancey tmancey deleted the issues/37589 branch April 15, 2024 21:49
@github-actions github-actions bot added this to the 1.67.x - Nightly milestone Apr 15, 2024
@btlechowski
Copy link

Verified with

Brave 1.67.14 Chromium: 124.0.6367.29 (Official Build) nightly (64-bit)
Revision 874b8ea
OS Linux

Verified recovered from corrupted state

[5816:5816:0416/123134.797683:VERBOSE1:database_manager.cc(177)] Migrated database from schema version 33 to schema version 36
[5816:5816:0416/123135.067187:VERBOSE1:legacy_client_migration.cc(74)] Migrating client state
[5816:5816:0416/123135.091383:VERBOSE3:legacy_client_migration.cc(88)] Successfully migrated client state
[5816:5816:0416/123135.092809:VERBOSE3:client_state_manager.cc(98)] Loading client state
[5816:5816:0416/123135.101466:VERBOSE3:client_state_manager.cc(496)] Successfully loaded client state
[0416/123135.138764:ERROR:elf_dynamic_array_reader.h(64)] tag not found
[0416/123135.154185:ERROR:file_io_posix.cc(145)] open /sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq: No such file or directory (2)
[0416/123135.154314:ERROR:file_io_posix.cc(145)] open /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq: No such file or directory (2)
[0416/123135.161098:ERROR:directory_reader_posix.cc(43)] opendir /home/bartlomiej/.config/BraveSoftware/Brave-Browser-Nightly/Crash Reports/attachments/8a0394d3-ce9a-4d6d-92a6-8efa2ae9d6ce: No such file or directory (2)
[5816:5816:0416/123135.175745:INFO:legacy_confirmation_migration.cc(64)] Confirmation state is corrupted, resetting to default values
[5816:5816:0416/123135.188719:VERBOSE1:legacy_confirmation_migration.cc(78)] Migrating confirmation state
[5816:5816:0416/123135.202706:VERBOSE9:legacy_confirmation_migration.cc(89)] Successfully saved confirmation state
[5816:5816:0416/123135.204485:VERBOSE3:confirmation_state_manager.cc(40)] Loading confirmation state
[5816:5816:0416/123135.206185:VERBOSE3:confirmation_state_manager.cc(66)] Successfully loaded confirmation state
[5816:5816:0416/123135.206813:VERBOSE1:ads_impl.cc(412)] Successfully initialized ads
[5816:5816:0416/123135.208198:VERBOSE1:account.cc(65)] Successfully initialized wallet

Verified ads are served:

[5816:5816:0416/123315.445392:VERBOSE1:notification_ad_handler.cc(168)] Opportunity arose to serve a notification ad
[5816:5816:0416/123315.445766:VERBOSE1:eligible_notification_ads_v2.cc(38)] Get eligible notification ads
[5816:5816:0416/123315.446870:VERBOSE1:user_idle_detection.cc(33)] User is active after 9 s
[5816:5816:0416/123315.450565:VERBOSE2:exclusion_rules_base.cc(100)] creativeSetId 45ea5952-37ca-41f0-88c2-4b546b0e6f4b has exceeded the perDay frequency cap
[5816:5816:0416/123315.453239:VERBOSE3:priority_util.h(21)] 9 untargeted ads with a priority of 1 in bucket 1
[5816:5816:0416/123315.453902:VERBOSE1:eligible_notification_ads_v2.cc(129)] Predicted ad with creative instance id 7e310987-0a02-46a4-86dc-262230dedb39 and a priority of 1
[5816:5816:0416/123315.454179:VERBOSE1:notification_ad_serving.cc(126)] Found 1 eligible ads
[5816:5816:0416/123315.454601:VERBOSE1:notification_ad_serving.cc(131)] Chosen eligible ad with creative instance id 7e310987-0a02-46a4-86dc-262230dedb39 and a priority of 1
[5816:5816:0416/123315.454896:VERBOSE1:notification_ad_handler.cc(184)] Served notification ad impression:
  placementId: b007ddb1-3621-4bab-9328-cfc0435d2def
  creativeInstanceId: 7e310987-0a02-46a4-86dc-262230dedb39
  creativeSetId: 47294a09-8053-4fb5-b171-531bec199a1e
  campaignId: 12e88c0d-fb1b-4316-ade0-cc06eb57227e
  advertiserId: 93130af3-2def-4ecb-b836-b3772e73b3c9
  segment: untargeted
  title: macros test
  body: test
  targetUrl: https://www.brave.com/?foo=bar&mtm_source=brave&&mtm_content=7e310987-0a02-46a4-86dc-262230dedb39
[5816:5816:0416/123315.464927:VERBOSE6:ads_service_impl.cc(848)] Timeout notification ad with placement id b007ddb1-3621-4bab-9328-cfc0435d2def in 120 s
[5816:5816:0416/123315.481513:VERBOSE3:notification_ad_handler.cc(195)] Served notification ad impression with placement id b007ddb1-3621-4bab-9328-cfc0435d2def and creative instance id 7e310987-0a02-46a4-86dc-262230dedb39

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ads] Follow up to #37390: Failing to Initialize ads due to corrupted confirmations.json
3 participants