Restrict PlatformViewCreationRequest constructor visibility#186019
Conversation
1. Restricted Constructor Visibility: * Changed the visibility of the primary PlatformViewCreationRequest constructor (with RequestedDisplayMode parameters) from public to private inside PlatformViewCreationRequest.java. * Enforced that all production callers must use the explicit, structured, mode-specific named static factory constructors (createHCPPRequest, createHybridCompositionRequest, createTLHCWithFallbackRequest). * Cleaned up the inline TODO(gmackall) comment inside the class declaration. 2. Preserved Testing Contracts: * Kept the @VisibleForTesting constructor public/package-private so that unit tests inside PlatformViewsControllerTest.java continue to compile and execute flawlessly. TAG=agy CONV=f1d2f12c-a509-41e0-b2da-e94d90b7a0bf
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
There was a problem hiding this comment.
Code Review
This pull request changes the visibility of the PlatformViewCreationRequest constructor from public to private to enforce the use of static factory methods. Feedback was provided to update the associated Javadoc, which currently suggests a preference for static factories rather than reflecting that they are now mandatory.
| * mode-specific named constructors above where possible. | ||
| */ | ||
| public PlatformViewCreationRequest( | ||
| private PlatformViewCreationRequest( |
There was a problem hiding this comment.
Now that this constructor is private, the Javadoc comment above (lines 127-129) is outdated. The phrase "Prefer use of the mode-specific named constructors" is no longer accurate since using the static factories is now mandatory. Please consider updating the documentation for clarity.
Pre-launch Checklist
///).