Summary: | Support for multiple speech enabled elements in same page. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Satish Sampath <satish> | ||||||||||
Component: | Forms | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, jorlow, steveblock | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 39485 | ||||||||||||
Attachments: |
|
Description
Satish Sampath
2010-08-12 09:00:49 PDT
Created attachment 64231 [details]
Patch
Comment on attachment 64231 [details]
Patch
WebCore/page/SpeechInput.h:74
+ HashMap<int, SpeechInputListener*> m_listeners;
Why do you need ids? Why not just keep track of listeners via their pointers?
Will you only be able to cancel recording from WebKit, and not cancel processing? If so, I think this might work since only one input box should ever be recording at once. In fact, if you get a message from one while another is recording, I think you can take that as a signal to end recording in the other. And that means only having one listener pointer at a time works fine. Why does everything ened IDs though? The multiple speech input boxes can all share the same backend since only one item will ever be listening at once...right?
If you can cancel processing from the controls, I guess it's more complicated because you could have multiple doing processing while another is recording and so on. In that case, I feel like you could have a pointer to the recording object and a set of processing objects. In that case, using IDs probably makes sense. But can't the IDs just be memory addresses? It seems a bit simpler than trying to dole out unique IDs.
Btw, can you try making a diagram (or even hand-drawing and scanning it in) of all the classes and how they interact and attach it to the top level bug?
> I guess it's more complicated because you could have multiple
> doing processing while another is recording and so on
That is correct, and this patch gets these cases working.
Jeremy and I discussed this more, and the map of IDs<->listeners seems like the right way to go so that if an input element gets destroyed and that memory was reallocated for another of the same type, we don't accidentally forward invalid callbacks to the new one.
Comment on attachment 64231 [details]
Patch
actually, leave it as ints
r=meWebCore/page/SpeechInput.cpp:57
+ for (HashMap<int, SpeechInputListener*>::iterator it = m_listeners.begin();
Maybe only do this in debug mode? Or at least assert not reached that you hit it?
Created attachment 64339 [details]
Patch
Added ASSERT_NOT_REACHED() per Jeremy.
Comment on attachment 64339 [details]
Patch
well...assert it's not reached and return...or put the whole thing in #if defined(DEBUG) and do an ASSERT(it->second == listener) instead.
Created attachment 64344 [details]
Patch
Placed the check under #ifdef debug and assert that the given listener is already present.
Comment on attachment 64344 [details]
Patch
WebCore/page/SpeechInput.cpp:59
+ it != m_listeners.end(); ++it)
sorry...one more nit....either put the whole for on one line or put {}'s around it...otherwise it's hard to follow.
Created attachment 64347 [details]
Patch
Move to a single line for loop (sorry, my bad..)
Comment on attachment 64347 [details]
Patch
r=me
Comment on attachment 64347 [details] Patch Clearing flags on attachment: 64347 Committed r65327: <http://trac.webkit.org/changeset/65327> All reviewed patches have been landed. Closing bug. |