Bug 92341 – Add message traces when media and plugins load or fail to load.

• Component: New Bugs
• Status: RESOLVED FIXED

[Jer Noble]

Add message traces when media and plugins load or fail to load.

[Jer Noble]

Created attachment 154561 [details]
Patch

[Early Warning System Bot]

Comment on attachment 154561 [details]
Patch

Attachment 154561 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13351563

[WebKit Review Bot]

Comment on attachment 154561 [details]
Patch

Attachment 154561 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13351556

[Early Warning System Bot]

Comment on attachment 154561 [details]
Patch

Attachment 154561 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13361409

[Gyuyoung Kim]

Comment on attachment 154561 [details]
Patch

Attachment 154561 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13342624

[Gustavo Noronha (kov)]

Comment on attachment 154561 [details]
Patch

Attachment 154561 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/13351583

[Eric Carlson]

Comment on attachment 154561 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154561&action=review

> Source/WebCore/html/HTMLMediaElement.cpp:1520
> +    default: return emptyString();

Shouldn't this have ASSERT_NOT_REACHED?

> Source/WebCore/html/HTMLMediaElement.cpp:1701
>          scheduleEvent(eventNames().loadeddataEvent);
>          setShouldDelayLoadEvent(false);
> +        if (document()->page() && document()->page()->settings()->messageTracingEnabled())
> +            document()->page()->chrome()->client()->traceMessage(ChromeClient::MediaLoaded, m_player->engineDescription());

I think it would be better to do this when we reach HAVE_METADATA, or an element with "preload=metadata" may not ever notify the client even thought it found and loaded part of a valid media file.

[Jer Noble]

(In reply to comment #7)
> (From update of attachment 154561 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=154561&action=review
> 
> > Source/WebCore/html/HTMLMediaElement.cpp:1520
> > +    default: return emptyString();
> 
> Shouldn't this have ASSERT_NOT_REACHED?

Sure.

> > Source/WebCore/html/HTMLMediaElement.cpp:1701
> >          scheduleEvent(eventNames().loadeddataEvent);
> >          setShouldDelayLoadEvent(false);
> > +        if (document()->page() && document()->page()->settings()->messageTracingEnabled())
> > +            document()->page()->chrome()->client()->traceMessage(ChromeClient::MediaLoaded, m_player->engineDescription());
> 
> I think it would be better to do this when we reach HAVE_METADATA, or an element with "preload=metadata" may not ever notify the client even thought it found and loaded part of a valid media file.

Is HAVE_METADATA guaranteed to only fire once?  (It looks like we will never drop down below HAVE_METADATA unless the user cancels load.  So yes.)

[Jer Noble]

Created attachment 154688 [details]
Patch

[WebKit Review Bot]

Comment on attachment 154688 [details]
Patch

Attachment 154688 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13351831

[Early Warning System Bot]

Comment on attachment 154688 [details]
Patch

Attachment 154688 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13345862

[Early Warning System Bot]

Comment on attachment 154688 [details]
Patch

Attachment 154688 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13342893

[Gyuyoung Kim]

Comment on attachment 154688 [details]
Patch

Attachment 154688 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13340887

[Jer Noble]

Committed r123780: <http://trac.webkit.org/changeset/123780>