Bug 145012 - Add internals setting to disable wireless playback availability for layout tests
Summary: Add internals setting to disable wireless playback availability for layout tests
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: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-14 11:59 PDT by Roger Fong
Modified: 2015-05-14 20:22 PDT (History)
3 users (show)

See Also:

Attachments
patch (5.43 KB, patch)
2015-05-14 12:04 PDT, Roger Fong 		no flags Details | Formatted Diff | Diff
patch (3.06 KB, patch)
2015-05-14 12:20 PDT, Roger Fong 		no flags Details | Formatted Diff | Diff
patch (3.06 KB, patch)
2015-05-14 12:50 PDT, Roger Fong 		eric.carlson: review+
Details | Formatted Diff | Diff
patch (20.97 KB, patch)
2015-05-14 12:59 PDT, Roger Fong 		no flags Details | Formatted Diff | Diff
patch (3.11 KB, patch)
2015-05-14 13:04 PDT, Roger Fong 		no flags Details | Formatted Diff | Diff
patch (3.10 KB, patch)
2015-05-14 13:22 PDT, Roger Fong 		commit-queue: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Roger Fong 2015-05-14 11:59:31 PDT 
<rdar://problem/20946504>
Comment 1 Roger Fong 2015-05-14 12:04:47 PDT 
Created attachment 253131 [details]
patch
Comment 2 Roger Fong 2015-05-14 12:05:10 PDT 
Doing this will allow local layout tests runs to be consistent with the bots.
Comment 3 Roger Fong 2015-05-14 12:20:41 PDT 
Created attachment 253132 [details]
patch
Comment 4 Roger Fong 2015-05-14 12:50:03 PDT 
Created attachment 253136 [details]
patch
Comment 5 Eric Carlson 2015-05-14 12:52:11 PDT 
Comment on attachment 253132 [details]
patch

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

> Source/WebCore/testing/InternalSettings.cpp:366
> +    page->settings().setMediaPlaybackAllowsAirPlay(false);

I assume always setting it to false is a copy/paste bug?

> Source/WebCore/testing/InternalSettings.h:128
> +    void setWirelessPlaybackDisabled(bool);

This needs to be guarded with WIRELESS_PLAYBACK_TARGET.
Comment 6 Eric Carlson 2015-05-14 12:52:55 PDT 
Comment on attachment 253136 [details]
patch

r=me with the fixes mentioned
Comment 7 Roger Fong 2015-05-14 12:59:26 PDT 
Created attachment 253137 [details]
patch

patch with fixes to verify builds
Comment 8 WebKit Commit Bot 2015-05-14 13:02:25 PDT 
Attachment 253137 [details] did not pass style-queue:


ERROR: LayoutTests/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Roger Fong 2015-05-14 13:04:40 PDT 
Created attachment 253138 [details]
patch
Comment 10 Roger Fong 2015-05-14 13:22:14 PDT 
Created attachment 253139 [details]
patch
Comment 11 WebKit Commit Bot 2015-05-14 14:04:15 PDT 
Comment on attachment 253139 [details]
patch

Rejecting attachment 253139 [details] from commit-queue.

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

/Volumes/Data/EWS/WebKit/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.appspot.com/results/5801458070978560
Comment 12 Roger Fong 2015-05-14 14:24:04 PDT 
http://trac.webkit.org/changeset/184351
Comment 13 Alexey Proskuryakov 2015-05-14 20:22:09 PDT 
Comment on attachment 253139 [details]
patch

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

> Source/WebCore/testing/InternalSettings.h:128
> +    void setWirelessPlaybackDisabled(bool);

A more common idiom is to have a function like setWirelessPlaybackEnabled(bool), to avoid double negation, or to have a function without an argument. I think that the latter is more appropriate in this state.