<html>
    <head>
      <base href="https://bugs.webkit.org/" />
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Add basic Media Session support to HTMLMediaElement"
   href="https://bugs.webkit.org/show_bug.cgi?id=145581#c12">Comment # 12</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Add basic Media Session support to HTMLMediaElement"
   href="https://bugs.webkit.org/show_bug.cgi?id=145581">bug 145581</a>
              from <span class="vcard"><a class="email" href="mailto:darin&#64;apple.com" title="Darin Adler &lt;darin&#64;apple.com&gt;"> <span class="fn">Darin Adler</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=254206&amp;action=diff" name="attach_254206" title="Patch (speculative Windows build fix)">attachment 254206</a> <a href="attachment.cgi?id=254206&amp;action=edit" title="Patch (speculative Windows build fix)">[details]</a></span>
Patch (speculative Windows build fix)

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=254206&amp;action=review">https://bugs.webkit.org/attachment.cgi?id=254206&amp;action=review</a>

<span class="quote">&gt; Source/WebCore/Modules/mediasession/HTMLMediaElementMediaSession.h:32
&gt; +#include &lt;wtf/text/WTFString.h&gt;</span >

Don’t need this.

<span class="quote">&gt; Source/WebCore/Modules/mediasession/HTMLMediaElementMediaSession.h:36
&gt; +class HTMLMediaElementMediaSession {</span >

What determines the interface and name of this class? It seems quite strange and I am not sure exactly what it’s for.

I might omit the second occurrence of the word “Media” but I’m not sure enough of context to know if that suggestion is OK.

<span class="quote">&gt; Source/WebCore/Modules/mediasession/HTMLMediaElementMediaSession.h:42
&gt; +    static const String&amp; kind(HTMLMediaElement* element) { return element-&gt;kind(); }
&gt; +    static void setKind(HTMLMediaElement* element, const String&amp; kind) { return element-&gt;setKind(kind); }
&gt; +
&gt; +    static MediaSession* session(HTMLMediaElement*, bool&amp;);
&gt; +    static void setSession(HTMLMediaElement* element, MediaSession* session) { element-&gt;setSession(session); }</span >

These should all take HTMLMediaElement&amp;, not HTMLMediaElement*.

<span class="quote">&gt; Source/WebCore/html/HTMLMediaElement.h:422
&gt; +    void setSession(MediaSession*);</span >

Might want this to take RefPtr&lt;MediaSession&gt;&amp;&amp; instead, if callers are handing over ownership. If ownership is usually shared, then raw pointer is OK/better.</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>