[Webkit-unassigned] [Bug 70897] Use a simple page client/controller for user consent in getUserMedia()
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Nov 8 22:56:32 PST 2011
https://bugs.webkit.org/show_bug.cgi?id=70897
--- Comment #11 from Adam Barth <abarth at webkit.org> 2011-11-08 22:56:31 PST ---
(From update of attachment 114178)
View in context: https://bugs.webkit.org/attachment.cgi?id=114178&action=review
Below are a few nits.
> Source/WebCore/mediastream/UserMediaController.cpp:90
> + DOMWindow* domWindow = request->frame()->existingDOMWindow();
> + if (!domWindow)
> + return;
> +
> + ScriptExecutionContext* context = domWindow->scriptExecutionContext();
> + if (!context || context != request->scriptExecutionContext())
> + return;
You should just get the ScriptExecutionContext from the frame without involving the DOMWindow:
ScriptExecutionContext* context = request->frame()->document();
> Source/WebCore/mediastream/UserMediaRequest.cpp:57
> + , m_scriptExecutionContext(frame->existingDOMWindow()->scriptExecutionContext())
Please don't go through the DOMWindow. frame->document() should work just find here.
> Source/WebCore/mediastream/UserMediaRequest.cpp:78
> + for (size_t i = 0; i < optionsList.size(); i++) {
++i
> Source/WebCore/mediastream/UserMediaRequest.cpp:80
> + optionsList[i].split(" ", subOptionsList);
Are you sure this is right? Usually these parsers use isHTMLSpace. Can you put a link the spec in the code?
> Source/WebCore/mediastream/UserMediaRequest.cpp:82
> + if (subOptionsList[0] == "audio")
This is really supposed to be case sensitive?
> Source/WebCore/mediastream/UserMediaRequest.cpp:99
> + m_frame = 0;
Do we need to zero out m_controller too?
> Source/WebCore/mediastream/UserMediaRequest.h:82
> + ScriptExecutionContext* m_scriptExecutionContext;
Does this need to be a RefPtr?
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list