Summary: | AX: CrashTracer: com.apple.WebKit.WebContent at WebKit: WebKit::WebSpeechSynthesisClient::speak | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | chris fleizach <cfleizach> | ||||||
Component: | Accessibility | Assignee: | chris fleizach <cfleizach> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, pvollan, rniwa, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
chris fleizach
2019-07-03 00:37:17 PDT
Created attachment 373396 [details]
patch
Comment on attachment 373396 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=373396&action=review > Source/WebKit/WebProcess/WebCoreSupport/WebSpeechSynthesisClient.cpp:57 > + if (auto observer = m_page.corePage()->speechSynthesisClient()->observer()) > + observer->didFinishSpeaking(); Could speechSynthesisClient() also possibly return nullptr? (In reply to Per Arne Vollan from comment #2) > Comment on attachment 373396 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=373396&action=review > > > Source/WebKit/WebProcess/WebCoreSupport/WebSpeechSynthesisClient.cpp:57 > > + if (auto observer = m_page.corePage()->speechSynthesisClient()->observer()) > > + observer->didFinishSpeaking(); > > Could speechSynthesisClient() also possibly return nullptr? I looked into it and to my eyes it did not seem so. It looks like it's set unconditionally Comment on attachment 373396 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=373396&action=review >>> Source/WebKit/WebProcess/WebCoreSupport/WebSpeechSynthesisClient.cpp:57 >>> + observer->didFinishSpeaking(); >> >> Could speechSynthesisClient() also possibly return nullptr? > > I looked into it and to my eyes it did not seem so. It looks like it's set unconditionally Perhaps it could return a reference, then? I don't think this is required in this patch, though. It can be done in a follow-up patch. Comment on attachment 373396 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=373396&action=review >>>> Source/WebKit/WebProcess/WebCoreSupport/WebSpeechSynthesisClient.cpp:57 >>>> + observer->didFinishSpeaking(); >>> >>> Could speechSynthesisClient() also possibly return nullptr? >> >> I looked into it and to my eyes it did not seem so. It looks like it's set unconditionally > > Perhaps it could return a reference, then? I don't think this is required in this patch, though. It can be done in a follow-up patch. let me look into that Comment on attachment 373396 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=373396&action=review >>>>> Source/WebKit/WebProcess/WebCoreSupport/WebSpeechSynthesisClient.cpp:57 >>>>> + observer->didFinishSpeaking(); >>>> >>>> Could speechSynthesisClient() also possibly return nullptr? >>> >>> I looked into it and to my eyes it did not seem so. It looks like it's set unconditionally >> >> Perhaps it could return a reference, then? I don't think this is required in this patch, though. It can be done in a follow-up patch. > > let me look into that m_speechSynthesisClient(WTFMove(pageConfiguration.speechSynthesisClient)) so if pageConfiguration.speechSynthesisClient is null, it could be null. I think we should add an extra null check for now. We can turn it to a reference in the future. r=me with an extra null check for speechSynthesisClient() everywhere. Created attachment 373557 [details]
patch
Comment on attachment 373557 [details] patch Clearing flags on attachment: 373557 Committed r247192: <https://trac.webkit.org/changeset/247192> All reviewed patches have been landed. Closing bug. |