Bug 220667

Summary:

Update media state for active speech recognition as it uses audio capture

Product:

WebKit

Reporter:

Sihui Liu <sihui_liu>

Component:

Media

Assignee:

Sihui Liu <sihui_liu>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, esprehn+autocc, ews-watchlist, kangil.han, webkit-bug-importer, youennf

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch for landing	none

Sihui Liu

Reported 2021-01-15 13:42:12 PST

...

Attachments
Patch (11.83 KB, patch) 2021-01-15 14:15 PST, Sihui Liu	no flags	Details Formatted Diff Diff
Patch (11.94 KB, patch) 2021-01-18 20:48 PST, Sihui Liu	no flags	Details Formatted Diff Diff
Patch (18.37 KB, patch) 2021-01-19 09:48 PST, Sihui Liu	no flags	Details Formatted Diff Diff
Patch for landing (17.81 KB, patch) 2021-01-19 19:37 PST, Sihui Liu	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Sihui Liu

Comment 1 2021-01-15 14:15:03 PST

Created attachment 417737 [details] Patch

youenn fablet

Comment 2 2021-01-18 09:38:44 PST

Comment on attachment 417737 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417737&action=review > Source/WebCore/Modules/speech/SpeechRecognition.cpp:112 > + auto& document = downcast<Document>(*scriptExecutionContext()); What is the guarantee to have a valid scriptExecutionContext() here? If we want to ensure that, we might want to remove SpeechRecognition from m_clientMap when being stopped. > Source/WebCore/Modules/speech/SpeechRecognition.cpp:142 > + document.setActiveSpeechRecognition(nullptr); Ditto.

Sihui Liu

Comment 3 2021-01-18 20:48:46 PST

Created attachment 417858 [details] Patch

Sihui Liu

Comment 4 2021-01-18 20:51:38 PST

(In reply to youenn fablet from comment #2) > Comment on attachment 417737 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=417737&action=review > > > Source/WebCore/Modules/speech/SpeechRecognition.cpp:112 > > + auto& document = downcast<Document>(*scriptExecutionContext()); > > What is the guarantee to have a valid scriptExecutionContext() here? > If we want to ensure that, we might want to remove SpeechRecognition from > m_clientMap when being stopped. Do you mean removing SpeechRecognition from m_clientMap when it is already stopped? How about just checking if the context is null (like in the updated patch)? > > > Source/WebCore/Modules/speech/SpeechRecognition.cpp:142 > > + document.setActiveSpeechRecognition(nullptr); > > Ditto.

youenn fablet

Comment 5 2021-01-19 02:03:09 PST

(In reply to Sihui Liu from comment #4) > (In reply to youenn fablet from comment #2) > > Comment on attachment 417737 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=417737&action=review > > > > > Source/WebCore/Modules/speech/SpeechRecognition.cpp:112 > > > + auto& document = downcast<Document>(*scriptExecutionContext()); > > > > What is the guarantee to have a valid scriptExecutionContext() here? > > If we want to ensure that, we might want to remove SpeechRecognition from > > m_clientMap when being stopped. > > Do you mean removing SpeechRecognition from m_clientMap when it is already > stopped? Right, idea would be to remove it proactively in SpeechRecognition::stop (the ActiveDOMObject version). Currently, this is done lazily: m_clientMap could in theory grow without ever decreasing. > How about just checking if the context is null (like in the updated patch)? Right, this would work as well.

Sihui Liu

Comment 6 2021-01-19 09:48:32 PST

Created attachment 417882 [details] Patch

Sihui Liu

Comment 7 2021-01-19 10:00:33 PST

(In reply to youenn fablet from comment #5) > (In reply to Sihui Liu from comment #4) > > (In reply to youenn fablet from comment #2) > > > Comment on attachment 417737 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=417737&action=review > > > > > > > Source/WebCore/Modules/speech/SpeechRecognition.cpp:112 > > > > + auto& document = downcast<Document>(*scriptExecutionContext()); > > > > > > What is the guarantee to have a valid scriptExecutionContext() here? > > > If we want to ensure that, we might want to remove SpeechRecognition from > > > m_clientMap when being stopped. > > > > Do you mean removing SpeechRecognition from m_clientMap when it is already > > stopped? > > Right, idea would be to remove it proactively in SpeechRecognition::stop > (the ActiveDOMObject version). Currently, this is done lazily: m_clientMap > could in theory grow without ever decreasing. I see, updated. > > > How about just checking if the context is null (like in the updated patch)? > > Right, this would work as well.

youenn fablet

Comment 8 2021-01-19 11:53:33 PST

Comment on attachment 417882 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417882&action=review > Source/WebCore/Modules/speech/SpeechRecognition.cpp:75 > + auto& document = downcast<Document>(*context); I am not sure this change is needed, we will anyway get a nullptr crash below. > Source/WebCore/Modules/speech/SpeechRecognition.h:89 > void suspend(ReasonForSuspension); final as well?

Sihui Liu

Comment 9 2021-01-19 19:37:30 PST

Created attachment 417939 [details] Patch for landing

EWS

Comment 10 2021-01-19 20:33:17 PST

Committed r271636: <https://trac.webkit.org/changeset/271636> All reviewed patches have been landed. Closing bug and clearing flags on attachment 417939 [details].

Radar WebKit Bug Importer

Comment 11 2021-01-19 20:34:15 PST

<rdar://problem/73387089>

Note You need to log in before you can comment on or make changes to this bug.