Bug 106354 - [EFL][GTK] Make the PulseAudioSanitizer an object on the EflPort, GtkPort
Summary: [EFL][GTK] Make the PulseAudioSanitizer an object on the EflPort, GtkPort
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-08 11:18 PST by Zan Dobersek
Modified: 2013-01-08 12:25 PST (History)
5 users (show)

See Also:


Attachments
Patch (11.32 KB, patch)
2013-01-08 12:06 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2013-01-08 11:18:12 PST
As Eric put it in https://bugs.webkit.org/show_bug.cgi?id=101261#c13
> But eventually we should fix that.  A port "is not" a PulseAudioSanitizer, it might "have a", but the usage of inheritance here is wrong IMO. :)

The EflPort and GtkPort interfaces currently inherit from PulseAudioSanitizer. This also makes the mocking of the latter in unit tests a bit clearer.
Comment 1 Zan Dobersek 2013-01-08 12:06:41 PST
Created attachment 181721 [details]
Patch
Comment 2 Eric Seidel (no email) 2013-01-08 12:14:33 PST
Comment on attachment 181721 [details]
Patch

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

Thanks.

> Tools/Scripts/webkitpy/layout_tests/port/efl.py:-52
>      def setup_test_run(self):
> -        self._unload_pulseaudio_module()

Should this be calling super?
Comment 3 Zan Dobersek 2013-01-08 12:17:18 PST
(In reply to comment #2)
> > Tools/Scripts/webkitpy/layout_tests/port/efl.py:-52
> >      def setup_test_run(self):
> > -        self._unload_pulseaudio_module()
> 
> Should this be calling super?

That method in layout_tests.port.base.Port currently does nothing, but it's still probably a good idea to call it.
https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py#L853

A new patch or can I just put the call in in this one? What works best for you?
Comment 4 Eric Seidel (no email) 2013-01-08 12:20:04 PST
(In reply to comment #3)
> (In reply to comment #2)
> > > Tools/Scripts/webkitpy/layout_tests/port/efl.py:-52
> > >      def setup_test_run(self):
> > > -        self._unload_pulseaudio_module()
> > 
> > Should this be calling super?
> 
> That method in layout_tests.port.base.Port currently does nothing, but it's still probably a good idea to call it.
> https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py#L853
> 
> A new patch or can I just put the call in in this one? What works best for you?

Doesn't matter to me in the slightest. :)  I'm happy to approve it either way.
Comment 5 Zan Dobersek 2013-01-08 12:23:20 PST
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > > Tools/Scripts/webkitpy/layout_tests/port/efl.py:-52
> > > >      def setup_test_run(self):
> > > > -        self._unload_pulseaudio_module()
> > > 
> > > Should this be calling super?
> > 
> > That method in layout_tests.port.base.Port currently does nothing, but it's still probably a good idea to call it.
> > https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py#L853
> > 
> > A new patch or can I just put the call in in this one? What works best for you?
> 
> Doesn't matter to me in the slightest. :)  I'm happy to approve it either way.

The ChromiumPort interface behaves the same way, so a new patch is more appropriate.
Comment 6 Zan Dobersek 2013-01-08 12:25:00 PST
Comment on attachment 181721 [details]
Patch

Clearing flags on attachment: 181721

Committed r139095: <http://trac.webkit.org/changeset/139095>
Comment 7 Zan Dobersek 2013-01-08 12:25:05 PST
All reviewed patches have been landed.  Closing bug.