Bug 190738 – [EME] Fix crash during tracing in gst_qtdemux_request_protection_context

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Charlie Turner]

And update new clearkey baselines.

[Charlie Turner]

Created attachment 352776 [details]
Patch

[Charlie Turner]

Created attachment 352777 [details]
Patch

[Charlie Turner]

Created attachment 352778 [details]
Patch

Rebase over master

[Charlie Turner]

Created attachment 352781 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 352781 [details]
Patch

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

> LayoutTests/platform/gtk/imported/w3c/web-platform-tests/encrypted-media/resources/clearkey-retrieve-destroy-persistent-license-expected.txt:10
> +layer at (0,0) size 800x600
> +  RenderView at (0,0) size 800x600
> +layer at (0,0) size 800x120
> +  RenderBlock {HTML} at (0,0) size 800x120
> +    RenderBody {BODY} at (8,8) size 784x104
> +      RenderBlock {DIV} at (0,0) size 784x0
> +      RenderBlock {DIV} at (0,0) size 784x104
> +        RenderText {#text} at (0,0) size 0x0
> +layer at (8,8) size 200x100
> +  RenderVideo {VIDEO} at (0,0) size 200x100

why are we getting the tree dump here?

> LayoutTests/platform/gtk/imported/w3c/web-platform-tests/encrypted-media/resources/clearkey-retrieve-persistent-license-expected.txt:10
> +layer at (0,0) size 800x600
> +  RenderView at (0,0) size 800x600
> +layer at (0,0) size 800x120
> +  RenderBlock {HTML} at (0,0) size 800x120
> +    RenderBody {BODY} at (8,8) size 784x104
> +      RenderBlock {DIV} at (0,0) size 784x0
> +      RenderBlock {DIV} at (0,0) size 784x104
> +        RenderText {#text} at (0,0) size 0x0
> +layer at (8,8) size 200x100
> +  RenderVideo {VIDEO} at (0,0) size 200x100

ditto

[Charlie Turner]

> why are we getting the tree dump here?

I don't know, it was generated from the run-webkit-tests script, which I was invoking like run-webkit-tests --gtk  --debug --test-list=$(fd '.*clearkey.*.html')

[Xabier Rodríguez Calvar]

Comment on attachment 352781 [details]
Patch

(In reply to Charlie Turner from comment #6)
> > why are we getting the tree dump here?
> 
> I don't know, it was generated from the run-webkit-tests script, which I was
> invoking like run-webkit-tests --gtk  --debug --test-list=$(fd
> '.*clearkey.*.html')

Then I think we should remove both these expectations and flag them because they shouldn't have a tree dump.

[Charlie Turner]

Created attachment 352790 [details]
Patch for landing

[Charlie Turner]

(In reply to Xabier Rodríguez Calvar from comment #7)
> Comment on attachment 352781 [details]
> Patch
> 
> (In reply to Charlie Turner from comment #6)
> > > why are we getting the tree dump here?
> > 
> > I don't know, it was generated from the run-webkit-tests script, which I was
> > invoking like run-webkit-tests --gtk  --debug --test-list=$(fd
> > '.*clearkey.*.html')
> 
> Then I think we should remove both these expectations and flag them because
> they shouldn't have a tree dump.

To be clear, the following tests where producing dump render tree output for unknown reasons. For now these have been marked as [ Missing ] in this patch.

        platform/gtk/imported/w3c/web-platform-tests/encrypted-media/resources/clearkey-retrieve-destroy-persistent-license.html
        platform/gtk/imported/w3c/web-platform-tests/encrypted-media/resources/clearkey-retrieve-persistent-license.html

[WebKit Commit Bot]

Comment on attachment 352790 [details]
Patch for landing

Rejecting attachment 352790 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 352790, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: https://webkit-queues.webkit.org/results/9666562

[Charlie Turner]

Created attachment 352791 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 352791 [details]
Patch for landing

Clearing flags on attachment: 352791

Committed r237292: <https://trac.webkit.org/changeset/237292>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Charlie Turner from comment #9)
> (In reply to Xabier Rodríguez Calvar from comment #7)
> > Comment on attachment 352781 [details]
> > Patch
> > 
> > (In reply to Charlie Turner from comment #6)
> > > > why are we getting the tree dump here?
> > > 
> > > I don't know, it was generated from the run-webkit-tests script, which I was
> > > invoking like run-webkit-tests --gtk  --debug --test-list=$(fd
> > > '.*clearkey.*.html')
> > 
> > Then I think we should remove both these expectations and flag them because
> > they shouldn't have a tree dump.
> 
> To be clear, the following tests where producing dump render tree output for
> unknown reasons. For now these have been marked as [ Missing ] in this patch.
> 
>        
> platform/gtk/imported/w3c/web-platform-tests/encrypted-media/resources/
> clearkey-retrieve-destroy-persistent-license.html
>        
> platform/gtk/imported/w3c/web-platform-tests/encrypted-media/resources/
> clearkey-retrieve-persistent-license.html

That's because they're not tests, they're helper files.

[Xabier Rodríguez Calvar]

(In reply to Ms2ger (he/him; ⌚ UTC+1/+2) from comment #14)> >        
> > platform/gtk/imported/w3c/web-platform-tests/encrypted-media/resources/
> > clearkey-retrieve-destroy-persistent-license.html
> >        
> > platform/gtk/imported/w3c/web-platform-tests/encrypted-media/resources/
> > clearkey-retrieve-persistent-license.html
> 
> That's because they're not tests, they're helper files.

So should we then skip them instead?

[Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Xabier Rodríguez Calvar from comment #15)
> (In reply to Ms2ger (he/him; ⌚ UTC+1/+2) from comment #14)> >        
> > > platform/gtk/imported/w3c/web-platform-tests/encrypted-media/resources/
> > > clearkey-retrieve-destroy-persistent-license.html
> > >        
> > > platform/gtk/imported/w3c/web-platform-tests/encrypted-media/resources/
> > > clearkey-retrieve-persistent-license.html
> > 
> > That's because they're not tests, they're helper files.
> 
> So should we then skip them instead?

I think that makes the most sense, yes.

[Miguel Gomez]

media/encrypted-media/clearKey/clearKey-webm-video-playback-mse.html was marked here as timeout, but it's passing reliably since r244585 (it seems), so updating expctations.

[Miguel Gomez]

media/encrypted-media/clearKey/clearKey-encrypted-webm-event-mse.html was marked failure here but since r244585 it has started to pass. Updating expectations.