Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix failure calling json function over cxxbridge #25898

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

cdesouza-chromium
Copy link
Collaborator

@cdesouza-chromium cdesouza-chromium commented Oct 9, 2024

Fix failure calling json function over cxxbridge

It is not entirely clear what is the underlying mechanism for this failure, and it should be investigated further. On Windows Debug builds, any calls to functions like json::convert_all_numbers_to_string were causing a runtime crash. Bisecting our branch has revealed that this crash started after an entirely unrelated change [1] that in fact seems quite anodyne.

This change correct the runtime failure with a minimum change.

Resolves brave/brave-browser#41497

[1] #25830

Resolves brave/brave-browser#41497

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:

@cdesouza-chromium cdesouza-chromium self-assigned this Oct 9, 2024
@cdesouza-chromium cdesouza-chromium requested a review from a team as a code owner October 9, 2024 00:01
@cdesouza-chromium cdesouza-chromium force-pushed the revert-gn-fix-breaking-json-rust-string branch from 2d107f4 to 6a076d5 Compare October 9, 2024 00:41
@cdesouza-chromium cdesouza-chromium changed the title Revert "fix gn check that broke again with #25799 (#25830)" Fix failure calling json function over cxxbridge Oct 9, 2024
@cdesouza-chromium cdesouza-chromium force-pushed the revert-gn-fix-breaking-json-rust-string branch 2 times, most recently from a3c762c to 96d52de Compare October 9, 2024 01:00
@cdesouza-chromium cdesouza-chromium force-pushed the revert-gn-fix-breaking-json-rust-string branch from 96d52de to 231324e Compare October 9, 2024 01:01
It is not entirely clear what is the underlying mechanism for this
failure, and it should be investigated further. On Windows Debug builds,
any calls to functions like `json::convert_all_numbers_to_string` were
causing a runtime crash. Bisecting our branch has revealed that this
crash started after an entirely unrelated change [1] that in fact seems
quite anodyne.

This change correct the runtime failure with a minimum change.

Resolves brave/brave-browser#41497

[1] #25830
@cdesouza-chromium cdesouza-chromium force-pushed the revert-gn-fix-breaking-json-rust-string branch from 231324e to 543bb38 Compare October 9, 2024 01:22
@cdesouza-chromium cdesouza-chromium merged commit 4e6b460 into master Oct 9, 2024
17 checks passed
@cdesouza-chromium cdesouza-chromium deleted the revert-gn-fix-breaking-json-rust-string branch October 9, 2024 11:29
@github-actions github-actions bot added this to the 1.72.x - Nightly milestone Oct 9, 2024
@brave-builds
Copy link
Collaborator

Released in v1.72.75

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
3 participants