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

[chromium] Add NullPointerException in PromptDelegateImpl #1268

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

mshin-wolvic
Copy link
Collaborator

This patch adds NullPointerException since mDelegate can be null.

Copy link
Member

@svillar svillar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I think it's better to add a null check instead of a try-catch. The reason is because having a null delegate is a valid code path more than an exceptional case, so I prefer to handle it as any other "normal" code path.

You could use this same PR to change the exception check that you added in #1256

This patch adds the null check since `mDelegate` can be null.
@mshin-wolvic mshin-wolvic force-pushed the add_safe_code_for_access_delegate branch from eeeebd2 to 4879b26 Compare February 22, 2024 11:15
@mshin-wolvic
Copy link
Collaborator Author

Personally I think it's better to add a null check instead of a try-catch. The reason is because having a null delegate is a valid code path more than an exceptional case, so I prefer to handle it as any other "normal" code path.

You could use this same PR to change the exception check that you added in #1256

Done

Copy link
Member

@svillar svillar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@svillar svillar merged commit 2c40573 into main Feb 22, 2024
20 checks passed
@svillar svillar deleted the add_safe_code_for_access_delegate branch February 22, 2024 15:48
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.

3 participants