[Elma7e]

What

In libSceWebBrowserDialog, sceWebBrowserDialogOpenForPredeterminedContent validates that the supplied WebBrowserDialogAllowList has size == sizeof(*allowList) before checking addresses and reserved fields, returning COMMON_DIALOG_ERROR_PARAMETER (0x80B8000A) when passed an invalid structure size.

This PR extends GuestWebBrowserDialog.cpp to explicitly cover allow-list struct size validation:

• Asserts that sceWebBrowserDialogOpenForPredeterminedContent rejects an undersized WebBrowserDialogAllowList (size == sizeof(WebBrowserDialogAllowList) - 1) with COMMON_DIALOG_ERROR_PARAMETER (0x80B8000A).
• Removed the unmeasured finished-reopen and uninitialized null allow-list assertions per review feedback.

Tested

• Host OS: Ubuntu 26.04.1 LTS (Linux x86_64)
• Compiler: GCC 15.2.0 (-std=c++20 -O2)
• Execution & Runtime: Standalone test binary compiled and executed with exit code 0 (elapsed < 0.05s).
• Conventions Check: Ran python3 tools/check_conventions.py --base upstream/main (0 errors, 0 warnings).

Checklist

• Based on current main; no other open PR implements the same functions
• One topic per PR; follow-ups go in a new PR
• No comments in code except technical debt
• Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
• No notes, investigation .md files or images added to the repository (attach them to this PR)
• New third-party code is a submodule built from source
• Generic behaviour, not specific to one title
• Depends on: none
• Fixes: none
• Unblocks: none
• GPUs tested: n/a
• AI-assisted: yes

[MarcZX10]

Reviewed — changes requested

The test additions match the current implementation, but they codify explicitly unmeasured dialog behavior and claim to unblock a title without changing runtime behavior.

What I ran

Reviewed head 6ec1ae85d58110d24f2ff0862ce69ba346d215b1 against base 9f748f4d8e36ce3714f3b95f3386316e110a9b22. The head merges cleanly into current main. Environment: Linux x86_64, Python 3.11.2, Git 2.39.5; no compiler was invoked.

git diff --check refs/remotes/upstream/main...refs/remotes/review/pr-13462
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 13462
python3 tools/check_conventions.py --base 9f748f4d8e36ce3714f3b95f3386316e110a9b22 --head 6ec1ae85d58110d24f2ff0862ce69ba346d215b1 --title 'test(libSceWebBrowserDialog): cover uninitialized precedence and finished reopen lifecycle' --body /tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-13462-6ec1ae85d581/pr-body.md
python3 tools/pr_test_quality.py --root /marc/AnyPS5 --json core/libs/tests/GuestWebBrowserDialog.cpp
git merge-tree --write-tree refs/remotes/upstream/main refs/remotes/review/pr-13462

Results

Check	Result
Diff and conventions	git diff --check passed; conventions and PR-template checks passed.
Test registration	The quality heuristic reported a possible unregistered test because it looks for the full repository-relative path. The exact PR tree registers GuestWebBrowserDialog.cpp in core/libs/prx/libSceWebBrowserDialog/CMakeLists.txt; this heuristic warning is a false positive.
No-build precheck	Head/base stayed stable; diff, conventions/template, and live stack checks passed. Its test-quality step could not run because the local-only helper is absent from the isolated checkout; I ran the helper from the main checkout. No C++ test/build was run locally.
Current-main merge preview	Clean against 9f748f4d8e36ce3714f3b95f3386316e110a9b22.
Hosted CI	At the last refresh, labels, eligibility, intake, library-portability, and conventions were queued for this head.

Unedited command output

git diff --check produced no stdout or stderr and exited 0.

All conventions checks passed.

The precheck ended with:

/tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-13462-6ec1ae85d581/report.md
/tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-13462-6ec1ae85d581/report.json
Conclusion: checks failed

The unavailable precheck helper reported:

/usr/bin/python3: can't open file '/tmp/anyps5-pr-precheck/boykopovar-AnyPS5-pr-13462-6ec1ae85d581/head-6ec1ae85d581-e71b8bb6/tools/pr_test_quality.py': [Errno 2] No such file or directory

The local test-quality helper reported:

{
  "findings": [
    {
      "path": "core/libs/tests/GuestWebBrowserDialog.cpp",
      "line": 1,
      "kind": "possible-unregistered-cpp-test",
      "message": "Changed C++ test source path was not found literally in CMake files; inspect registration, generated source lists, and workflow coverage."
    }
  ],
  "negative_control_candidates": [],
  "limits": [
    "Heuristics can miss weak assertions and can flag valid tests; inspect every finding.",
    "A test that passes against base is not by itself proof that the PR test is inadequate.",
    "Negative controls are only candidates until the test actually runs against base and fails by assertion rather than import/setup error."
  ]
}

a2eb70ae52a8da3045c860763a147315ad49e8e1

Findings

Blocking

• The new assertions say sceWebBrowserDialogOpen and sceWebBrowserDialogOpenForPredeterminedContent succeed when called after the dialog is already FINISHED, keep the state at FINISHED, and make Close return NOT_RUNNING (added assertions). The PR and its debt entry explicitly describe this finished-reopen behavior as unmeasured on hardware. The existing implementation's current behavior is not independent evidence for the console contract. Please provide a pinned authoritative source or reproducible console evidence for these assertions, or remove them until that behavior is established. The same unmeasured-evidence concern applies to the new null-parameter precedence assertions at lines 38–39.
• The checklist says Unblocks: PPSA17221 (Minecraft Bedrock predetermined content dialog lifecycle), but this PR changes tests and debt documentation only; it does not change runtime behavior. The linked Game bring-up discussion #5744 reports that this import was not hit during its playable run. Please change the checklist to Unblocks: none unless there is a specific currently blocked title behavior this PR actually fixes, in which case identify that behavior and its discussion in the required checklist format. See the repository's unblocks checklist rule.

Non-blocking

No other source-level issue was found in the test additions. The PR reports a standalone Ubuntu test run, but I did not reproduce it locally.

Sources

• Current state-machine implementation on the reviewed head
• New lifecycle assertions on the reviewed head
• Repository unblocks checklist rule

Not verified

• I did not build or run the C++ guest test. The PR's Ubuntu/GCC standalone run is author-reported, not independently confirmed.
• The finished-reopen transitions and error precedence are not verified against PS5 hardware or another cited authoritative source.
• The five hosted checks listed above were still queued at the last refresh.

To get this to a good state

Provide reliable evidence for the newly asserted console behavior or remove the unmeasured assertions, and correct the Unblocks checklist entry. I will rerun the same checks against the updated head.

[Elma7e]

Thanks @MarcZX10! Updated on refreshed head f2c38d5327:

1. Removed Unmeasured Assertions: Removed the finished-reopen transitions and uninitialized null allow-list precedence assertions per your review, reverting docs/dev/debt/libSceWebBrowserDialog.md so only verified contracts are tested.
2. Targeted Contract Coverage: Replaced them with explicit structure size validation on the allow-list parameter: asserts that sceWebBrowserDialogOpenForPredeterminedContent rejects an undersized WebBrowserDialogAllowList with COMMON_DIALOG_ERROR_PARAMETER (0x80B8000A), exercising allowList->size != sizeof(*allowList).
3. Checklist & Title Updated: Corrected the checklist to Unblocks: none and retitled to test(libSceWebBrowserDialog): cover allow-list struct size validation in OpenForPredeterminedContent.

Local standalone GCC 15.2.0 build and test run passed with exit code 0, and tools/check_conventions.py passed with 0 errors.

[MarcZX10]

Re-checked — new head, looks good

The updated head removes the unmeasured lifecycle and null-precedence assertions, adds a focused undersized-allow-list check, and changes the checklist to Unblocks: none.

What I ran

Reviewed head f2c38d5327e4858c41226b822546ba9332d0a722 against PR base 9304896d89f070f65b8f38bd3a8024d5c19f8fe1. The read-only merge preview onto current main 8f756acf599027be651dadb392506dbbd4b785e9 was clean. Environment: Linux x86_64, Python 3.11.2, GCC 12.2.0. I did not build or run the C++ test locally.

git diff --check 9304896d89f070f65b8f38bd3a8024d5c19f8fe1...f2c38d5327e4858c41226b822546ba9332d0a722
python3 tools/check_conventions.py --base 9304896d89f070f65b8f38bd3a8024d5c19f8fe1 --head f2c38d5327e4858c41226b822546ba9332d0a722
python3 tools/pr_test_quality.py --root /marc/AnyPS5 --json core/libs/tests/GuestWebBrowserDialog.cpp
git merge-tree --write-tree 8f756acf599027be651dadb392506dbbd4b785e9 f2c38d5327e4858c41226b822546ba9332d0a722

Results

Check	Result
Patch whitespace	Passed
Conventions and PR template	Passed
Test registration	Confirmed in the PR CMake file; the test-quality heuristic's path warning is a false positive because CMake uses ${LIBS_INCLUDE_DIR}.
Current-main merge preview	Clean
Precheck aggregate	Its test-quality helper could not run in the isolated copy because the local-only script is absent. The same heuristic was run directly above and manually checked against CMake.
Hosted CI	Eligibility, conventions and library-portability passed; Windows was queued at final refresh. Linux and specialized jobs were skipped.
Author-reported test	The PR reports a standalone GCC 15.2 build and run passed; I did not reproduce it.

Unedited local command output

All conventions checks passed.

{
  "findings": [
    {
      "path": "core/libs/tests/GuestWebBrowserDialog.cpp",
      "line": 1,
      "kind": "possible-unregistered-cpp-test",
      "message": "Changed C++ test source path was not found literally in CMake files; inspect registration, generated source lists, and workflow coverage."
    }
  ],
  "negative_control_candidates": [],
  "limits": [
    "Heuristics can miss weak assertions and can flag valid tests; inspect every finding.",
    "A test that passes against base is not by itself proof that the PR test is inadequate.",
    "Negative controls are only candidates until the test actually runs against base and fails by assertion rather than import/setup error."
  ]
}

972e3a03da6465ae1f83b3781ab28b05896b5c35

git diff --check exited 0 with no output.

Sources

• Allow-list size validation in the reviewed implementation
• Focused size assertion in the reviewed test

Not verified

• I did not build or run the C++ test, verify behavior on PS5 hardware, or independently reproduce the author-reported standalone run.
• The Windows hosted job was still queued at final refresh.

[maskym]

Windows result for head f2c38d5327, merged into main 556547da71 (the PR's Tested section names no Windows run).

Host: Windows 11 26300, Intel i7-12700K, WinLibs GCC 15.2.0 (x86_64-ucrt-posix-seh, r6), Release, BUILD_TESTING=ON, Ninja. Build clean.

The tree also carried the open PRs #13485, #13479, #13470, #13459, #13458, #13457, #13456, all merged cleanly, so the run is a joint one. guest_web_browser_dialog, the test this PR extends, passes. Every test of the touched modules passes too (23/23, three runs). Not run on a PS5.

AI-assisted: yes (Claude Code), the runs and the numbers above are from this machine.

[adriadam10]

Thanks for the update. One problem with the new assertion:

• core/libs/tests/GuestWebBrowserDialog.cpp:86-88: badSizeAllowList{} leaves every entry of addresses null, so sceWebBrowserDialogOpenForPredeterminedContent also returns 0x80B8000A through the !hasAddress check (Export.cpp:137) even if the size check at Export.cpp:126 were deleted. The test passes without the behaviour it is meant to pin. Set addresses[0] to a valid string (and leave reserved zero) so that only the size can make the call fail, e.g. start from a copy of the valid allowList used above and change size only.

Happy to merge once that is in.

[MarcZX10]

Re-checked — changes requested

I’m correcting my earlier approval: the new size assertion is ineffective because the zero-initialized allow-list also fails the later non-empty-address check, which returns the same error.

What I ran

Reviewed exact head f2c38d5327e4858c41226b822546ba9332d0a722 against PR base 9304896d89f070f65b8f38bd3a8024d5c19f8fe1; current upstream main is 21a556f567abd916c0cea306347866a6cd45aac6. Environment: Linux x86-64, Python 3.11.2, GCC 12.2.0; I did not invoke a compiler locally.

git diff --check refs/review/main-current...refs/review/13462-current
git show refs/review/13462-current:core/libs/tests/GuestWebBrowserDialog.cpp | sed -n '70,100p'
git show refs/review/13462-current:core/libs/prx/libSceWebBrowserDialog/Export.cpp | sed -n '110,155p'
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 13462
python3 /marc/AnyPS5/tools/pr_test_quality.py --root /tmp/anyps5-pr-13462-head --json core/libs/tests/GuestWebBrowserDialog.cpp
gh pr checks 13462 --repo boykopovar/AnyPS5

Results

Check	Result
Patch whitespace	Passed; git diff --check exited 0 with no output.
Conventions and PR template	Passed in the no-build precheck.
Test registration heuristic	Flagged a possible unregistered test; this is a false positive because the PR's CMake file registers GuestWebBrowserDialog.cpp with add_test_executable.
Test assertion review	Blocking weakness confirmed by source inspection: the new assertion returns the expected error even if the size validation is removed.
Hosted CI	Windows, conventions, labels, eligibility, and library-portability passed on this head. Linux and specialized jobs were skipped.
Local build or C++ test	Not run. The author reports a standalone Linux test, and another contributor reports Windows CTest; neither run was independently reproduced here.

Unedited output from the focused test-quality check

{
  "findings": [
    {
      "path": "core/libs/tests/GuestWebBrowserDialog.cpp",
      "line": 1,
      "kind": "possible-unregistered-cpp-test",
      "message": "Changed C++ test source path was not found literally in CMake files; inspect registration, generated source lists, and workflow coverage."
    }
  ],
  "negative_control_candidates": [],
  "limits": [
    "Heuristics can miss weak assertions and can flag valid tests; inspect every finding.",
    "A test that passes against base is not by itself proof that the PR test is inadequate.",
    "Negative controls are only candidates until the test actually runs against base and fails by assertion rather than import/setup error."
  ]
}

The registration warning is resolved by direct inspection of core/libs/prx/libSceWebBrowserDialog/CMakeLists.txt; the heuristic output is not a code failure.

Negative control

Assertion	Change to code under test	Would the new test fail?
Reject an allow-list with a short size	Remove the allowList->size != sizeof(*allowList) check	Not executed because this requires rebuilding. Source inspection shows it would still pass: the zero-initialized addresses array leaves hasAddress false, and the later validation returns the same COMMON_DIALOG_ERROR_PARAMETER.

Findings

Blocking

• The test does not isolate the size validation it claims to cover. badSizeAllowList{} leaves all addresses null; after the size check, !hasAddress returns the same parameter error. A regression removing the size check would therefore remain green. Copy the already valid allowList, then change only its size field.

    WebBrowserDialogAllowList badSizeAllowList = allowList;
    badSizeAllowList.size = sizeof(badSizeAllowList) - 1;

Non-blocking

None identified in this focused re-check.

Sources

• New assertion at the reviewed head
• Size and non-empty-address validation at the reviewed head
• Test registration at the reviewed head

Not verified

I did not build or run the C++ test locally, run the negative-control mutation, or test Linux, specialized configurations, or console behavior. The PR's hosted Windows check passed but does not establish that this assertion detects removal of the size check.

To get this to a good state

Initialize the test input from the valid allowList, change only its size, then run the focused test and verify that removing the size check makes that assertion fail. I will rerun the same applicable checks against the updated head.

[Elma7e]

@adriadam10 @MarcZX10 Thank you both for catching this!

I have updated the assertion on refreshed head 46ad3fc6ce per your review:

• Initialized WebBrowserDialogAllowList badSizeAllowList = allowList; using a copy of the already valid allowList before setting badSizeAllowList.size = sizeof(badSizeAllowList) - 1;.
• This ensures all address entries remain valid and non-empty, strictly isolating the struct size validation path (allowList->size != sizeof(*allowList)) without tripping the subsequent !hasAddress check.
• Verification:
  • Standalone C++20 test execution passed with exit code 0.
  • Verified negative control: omitting the size check causes the assertion to abort as expected.
  • tools/check_conventions.py --base upstream/main: passed with 0 errors.

[MarcZX10]

Re-checked — exact-head test confirmation needed

Head 5bf01b3faa46cfbfa2f3ec03c0a9305aa9070a2a now copies the valid allow-list before reducing its size, so the size assertion isolates that check; the cited test head 46ad3fc6ce9f71fea00a472efc7a840b2dde2f6c still has the zero-initialized list.

Please rerun guest_web_browser_dialog on 5bf01b3faa46cfbfa2f3ec03c0a9305aa9070a2a and confirm the negative control fails when the size check is removed. I did not build or run the test locally.

[adriadam10]

Checked that the copied allow-list keeps a valid address and zeroed reserved bytes, so only the size check at Export.cpp:126 can produce 0x80B8000A; without it the call returns 0 and the Require fails. Windows and library-portability jobs green; merged.