[Webkit-unassigned] [Bug 21562] Layering violation: MediaPlayer should not reference/use FrameView

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=21562





--- Comment #10 from Carlos Garcia Campos <cgarcia at igalia.com>  2014-10-08 22:40:37 PST ---
(In reply to comment #9)
> (From update of attachment 239020 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=239020&action=review
> 
> >>> Source/WebCore/platform/graphics/MediaPlayer.h:267
> >>> +    virtual bool mediaPlayerInMediaDocument() const { return false; }
> >> 
> >> I’m not so fond of this name, because it sounds like a function that returns “a media player that is in a media document” rather than a function that answers a question about whether the media player is in a media document. I guess maybe the mediaPlayer is a prefix we use on a lot of function names. Maybe mediaPlayerIsInMediaDocument would be better.
> >> 
> >> But also, maybe there’s a more abstract way to describe this. It seems like a concept layering violation for the media player to even have a concept of “in media document”, so it would be nice to change this concept to instead express what behavior we expect from the media player rather than the situation.
> > 
> > Yes, all virtual methods in MediaPlayerClient use the mediaPlayer prefix.
> 
> Please read the rest of my comment, though. That was only the first thing, but the commend said three things. For example, I suggested that mediaPlayerIsInMediaDocument would be a better name.

Sure, I read that, but didn't comment anything about the name because I agreed with your suggestions. I'll submit an updated patch anyway.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.