1266035 - Move AvailableIn=(PrivilegedApps|CertifiedApps) to ChromeOnly

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[:gerard-majax]

Do we want to do this for all APIs?

Comment 1

[:gerard-majax]

And we want to make sure we do not start exposing those in browser.

Comment 2

[:gerard-majax]

Boris, history shows you are by far the biggest contributor to dom/bindings/Codegen.py and related so I guess you have an opinion on this: should I completely get rid of AvailableIn and CheckAnyPermission logic, or should I just remove support for PrivilegedApps and CertifiedApps ?

I would tend to think the former makes more sense, because I don't see any other current use outside of B2G.

Comment 3

[Boris Zbarsky [:bzbarsky]]

This stuff was added entirely for b2g I believe.  Jonas, do we have any plans to reuse it for something else, or should it just go away?

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I can't speak for B2G/CD plans any more. Forwarding to Fabrice.

Comment 5

[:gerard-majax]

I could get Mulet debug booting transition Gaia branch with no visible regression :).

Comment 6

[[:fabrice] Fabrice Desré]

No plans to reuse that, so no objections on my side to remove all that support.

Comment 7

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #5)
> I could get Mulet debug booting transition Gaia branch with no visible
> regression :).

And on device.

Meanwhile: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e0a794dbdf2

Comment 8

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #7)
> (In reply to Alexandre LISSY :gerard-majax from comment #5)
> > I could get Mulet debug booting transition Gaia branch with no visible
> > regression :).
> 
> And on device.
> 
> Meanwhile:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e0a794dbdf2

Much failures because we do not expose the API, in mochitests, I guess this is a good sign :)

Comment 9

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #7)
> (In reply to Alexandre LISSY :gerard-majax from comment #5)
> > I could get Mulet debug booting transition Gaia branch with no visible
> > regression :).
> 
> And on device.
> 
> Meanwhile:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e0a794dbdf2

And for reference, a push without the patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=49ff346eca8f

Comment 10

[:gerard-majax]

Attachment: Get rid of AvailableIn, CheckAnyPermissions and CheckAllPermissions r?bz

This is good enough to move exposure to ChromeOnly and builds everywhere.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8745304 [details] [diff] [review]
Get rid of AvailableIn, CheckAnyPermissions and CheckAllPermissions r?bz

>     def FeatureList(config):

So... with these changes, things always ends up being the empty set/list, no?  Seems like there's more stuff to remove here, possibly in a followup.

Similarly, dom::IsInCertifiedApp and dom::IsInPrivilegedApp are now dead code, I believe, along with the stuff in workers tracking that state...  Again, followup is probably OK.

r=me, but do make sure that various dead code gets removed.

Comment 12

[:gerard-majax]

I will verify and include those changes now. Thanks!

Comment 13

[:gerard-majax]

Removed those dead code in the python code generator, the dom::IsInCertifiedApp/IsInPrivilegedApp and the subsequents parts within worker code. Now checking how it goes with building/running :)

Comment 14

[:gerard-majax]

Attachment: Get rid of AvailableIn, CheckAnyPermissions and CheckAllPermissions

Updated patch, with the extra dead code removed. How should I proceed for
reviewing the WebIDL changes? Should I ask sr to you on all of them, or just
ask review to the people in charge of each one?

Comment 15

[Boris Zbarsky [:bzbarsky]]

Can you please post the interdiff against the patch I already reviewed?  Bugzilla's diff, as usual, is failing.  Or at least claims it is.

For the IDL changes, probably best to get reviews from people who own the relevant APIs; I can do a once-over.

Comment 16

[:gerard-majax]

pure pine: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e80062506e79
this bug: https://treeherder.mozilla.org/#/jobs?repo=try&revision=564f32ea88ea

Comment 17

[:gerard-majax]

Interdiff at https://bugzilla.mozilla.org/attachment.cgi?oldid=8745304&action=interdiff&newid=8747104&headers=1 is working for me and looks legit, I'll generate one from my tree :)

Comment 18

[:gerard-majax]

Attachment: bug1266035_inter.diff

This should be the good interdiff

Comment 19

[Boris Zbarsky [:bzbarsky]]

> Interdiff at https://bugzilla.mozilla.org/attachment.cgi?oldid=8745304&action=interdiff&newid=8747104&headers=1 is working for me 

Well, except for the part where it claims it's probably wrong, yes?  ;)

Thanks for putting up an interdiff.

Comment 20

[:gerard-majax]

Attachment: Turn RIL-related APIs to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: B0PPS6HCrgx

Comment 21

[:gerard-majax]

Attachment: Turn TV-related APIs to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: KxZIOO3OXsK

Comment 22

[:gerard-majax]

Attachment: Turn mozAlarms API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: IHoxG2y32CL

Comment 23

[:gerard-majax]

Attachment: Turn mozApps API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: Hn0Yueqj0CD

Comment 24

[:gerard-majax]

Attachment: Turn Input-related APIs to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: 8LuaE2V0fx1

Comment 25

[:gerard-majax]

Attachment: Turn Bluetooth-related APIs to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: 5rCY03CLBaG

Comment 26

[:gerard-majax]

Attachment: Turn NFC-related APIs to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: 7j3MCq5b6Zg

Comment 27

[:gerard-majax]

Attachment: Turn RequestSync API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: DmCbtaSxgFe

Comment 28

[:gerard-majax]

Attachment: Turn Download API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: 3n3gUhRVMoF

Comment 29

[:gerard-majax]

Attachment: Turn Settings API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: 7SCYk7CoyEd

Comment 30

[:gerard-majax]

Attachment: Turn Contacts API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: L2C0ILv60Hr

Comment 31

[:gerard-majax]

Attachment: Turn NetworkStats API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: AIwTszgegor

Comment 32

[:gerard-majax]

Attachment: Turn ResourceStats API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: JOnHviptd2p

Comment 33

[:gerard-majax]

Attachment: Turn BrowserElement API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: CRXiEKkmipp

Comment 34

[:gerard-majax]

Attachment: Turn UDP/TCP Socket APIs to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: 5CmuER0FJCy

Comment 35

[:gerard-majax]

Attachment: Turn mozPower API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: KIYliqNH0BI

Comment 36

[:gerard-majax]

Attachment: Turn Feature API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: 9FYhjxhcyUP

Comment 37

[:gerard-majax]

Attachment: Turn IdleObserver API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: F4sq3m1j6WA

Comment 38

[:gerard-majax]

Attachment: Turn mozFMRadio API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: 7QZ0Mp22r9Y

Comment 39

[:gerard-majax]

Attachment: Turn mozTime API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: Eba5Qo8YDfB

Comment 40

[:gerard-majax]

Attachment: Turn SystemUpdate API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: JHHa8YCUBrc

Comment 41

[:gerard-majax]

Attachment: Turn AudioChannelManager API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: AVbgauvTDlR

Comment 42

[:gerard-majax]

Attachment: Turn mozChromeNotifications API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: EqiDAECJ2Sj

Comment 43

[:gerard-majax]

Attachment: Turn EngineeringMode API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: M7fGGOOTW6

Comment 44

[:gerard-majax]

Attachment: Turn mozKillSwitch API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: LocqWgykUYI

Comment 45

[:gerard-majax]

Attachment: Turn mozTetheringManager API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: 8pwXD8ZtprJ

Comment 46

[:gerard-majax]

Attachment: Turn SimplePush API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: HoNh8mPESax

Comment 47

[:gerard-majax]

Plan: fix tests, ask r? and sr? after.

Comment 48

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8747122 [details] [diff] [review]
bug1266035_inter.diff

Awesome.  r=me

Comment 49

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8747104 [details] [diff] [review]
Get rid of AvailableIn, CheckAnyPermissions and CheckAllPermissions

r=me

Comment 50

[:gerard-majax]

Attachment: Switch TV-related mochitests to chrome

MozReview-Commit-ID: C2ezlZB9z0a

Comment 51

[:gerard-majax]

Attachment: Turn TV-related APIs to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: KxZIOO3OXsK

Comment 52

[:gerard-majax]

Attachment: Turn Contacts API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: L2C0ILv60Hr

Comment 53

[:gerard-majax]

Attachment: Switch Contacts-related mochitests to chrome

MozReview-Commit-ID: F5nI61L9rYe

Comment 54

[:gerard-majax]

Attachment: Turn Feature API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: 9FYhjxhcyUP

Comment 55

[:gerard-majax]

Attachment: Switch Feature-related mochitests to chrome

MozReview-Commit-ID: 5jgIsXVtHOL

Comment 56

[:gerard-majax]

Comment on attachment 8748077 [details] [diff] [review]
Switch Contacts-related mochitests to chrome

Review of attachment 8748077 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bindings/test/chrome.ini
@@ +13,1 @@
>  [test_bug707564-chrome.html]

And test_bug707564-chrome.html is failing (because of http:// loaded from chrome://, fixing this then it fails on wrappedJSObjet undefined), should we just kill it? It feels like just dupe now.

Comment 57

[:gerard-majax]

Attachment: Switch Contacts-related mochitests to chrome

MozReview-Commit-ID: F5nI61L9rYe

Comment 58

[:gerard-majax]

Attachment: Switch mozChromeNotifications mochitests to chrome

MozReview-Commit-ID: EFmqrkGXjms

Comment 59

[:gerard-majax]

Attachment: Turn mozChromeNotifications API to ChromeOnly r=fabrice sr=bz

MozReview-Commit-ID: EqiDAECJ2Sj

Comment 60

[:gerard-majax]

Attachment: Switch Contacts-related mochitests to chrome

MozReview-Commit-ID: F5nI61L9rYe

Comment 61

[:gerard-majax]

Comment on attachment 8747206 [details] [diff] [review]
Turn RequestSync API to ChromeOnly r=fabrice sr=bz

Dead API

Comment 62

[:gerard-majax]

Comment on attachment 8747223 [details] [diff] [review]
Turn mozKillSwitch API to ChromeOnly r=fabrice sr=bz

(soon) Dead API

Comment 63

[:gerard-majax]

Attachment: Turn TV-related APIs to ChromeOnly r=fabrice sr=bz

Pref for Presentation DeviceInfo was removed. Restore it.

Comment 64

[:gerard-majax]

Attachment: Switch Contacts-related mochitests to chrome

Comment 65

[:gerard-majax]

Current status: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2873f0f5542
Pine reference: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9759f72baeb8

Comment 66

[:gerard-majax]

And pine as reference before removing RequestSync/KillSwitch APIs: https://treeherder.mozilla.org/#/jobs?repo=try&revision=62ecac29b6e4

Comment 67

[:gerard-majax]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb6e206c345b

Comment 68

[:gerard-majax]

Biggest pain point to deal with is going to be the mochitest-oop file from dom/browser-element/

Comment 69

[:gerard-majax]

With webidl changes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b754f9af9ef
Without those: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b3b8c3a4d01bffd53f41619aa38acfac703d57e

We are getting to the point of all tests migrated and/or a few of them disabled (mostly mochitest-oop that are indeed testing B2G specific features). I am in the process of writing a tool to compare test runs per platforms on each try push to ensure we have not reduced the coverage (at least that any change is what we meant).

Comment 70

[:gerard-majax]

Attachment: Turn RIL-related APIs to ChromeOnly r=fabrice sr=bz

Comment 71

[:gerard-majax]

Attachment: Switch RIL-related mochitests to chrome

Comment 72

[:gerard-majax]

Attachment: Turn TV-related APIs to ChromeOnly r=fabrice sr=bz

Comment 73

[:gerard-majax]

Attachment: Switch TV-related mochitests to chrome

Comment 74

[:gerard-majax]

Attachment: Switch TV-related mochitests to chrome

Comment 75

[:gerard-majax]

Attachment: Turn mozAlarms API to ChromeOnly r=fabrice sr=bz

Comment 76

[:gerard-majax]

Attachment: Turn mozApps API to ChromeOnly r=fabrice sr=bz

Comment 77

[:gerard-majax]

Attachment: Turn Input-related APIs to ChromeOnly r=fabrice sr=bz

Comment 78

[:gerard-majax]

Attachment: Switch Input-related mochitests to chrome

Comment 79

[:gerard-majax]

Attachment: Disable some inputmethod test that are failing

Comment 80

[:gerard-majax]

Attachment: Turn Bluetooth-related APIs to ChromeOnly r=fabrice sr=bz

Comment 81

[:gerard-majax]

Attachment: Turn NFC-related APIs to ChromeOnly r=fabrice sr=bz

Comment 82

[:gerard-majax]

Attachment: Turn Download API to ChromeOnly r=fabrice sr=bz

Comment 83

[:gerard-majax]

Attachment: Turn Settings API to ChromeOnly r=fabrice sr=bz

Comment 84

[:gerard-majax]

Attachment: Switch mozSettings mochitests to chrome

Comment 85

[:gerard-majax]

Attachment: Turn Contacts API to ChromeOnly r=fabrice sr=bz

Comment 86

[:gerard-majax]

Attachment: Switch Contacts-related mochitests to chrome

Comment 87

[:gerard-majax]

Attachment: Turn NetworkStats API to ChromeOnly r=fabrice sr=bz

Comment 88

[:gerard-majax]

Attachment: Turn ResourceStats API to ChromeOnly r=fabrice sr=bz

Comment 89

[:gerard-majax]

Attachment: Turn BrowserElement API to ChromeOnly r=fabrice sr=bz

Comment 90

[:gerard-majax]

Attachment: Switch BrowserElement-related mochitests to chrome

Comment 91

[:gerard-majax]

Attachment: Disable some mochitest-oop tests

Comment 92

[:gerard-majax]

Attachment: Turn UDP/TCP Socket APIs to ChromeOnly r=fabrice sr=bz

Comment 93

[:gerard-majax]

Attachment: Switch UDP/TCP Socket mochitests to chrome

Comment 94

[:gerard-majax]

Attachment: Turn mozPower API to ChromeOnly r=fabrice sr=bz

Comment 95

[:gerard-majax]

Attachment: Switch mozPower mochitests to chrome

Comment 96

[:gerard-majax]

Attachment: Turn Feature API to ChromeOnly r=fabrice sr=bz

Comment 97

[:gerard-majax]

Attachment: Switch Feature-related mochitests to chrome

Comment 98

[:gerard-majax]

Attachment: Turn IdleObserver API to ChromeOnly r=fabrice sr=bz

Comment 99

[:gerard-majax]

Attachment: Switch IdleObserver mochitests to chrome

Comment 100

[:gerard-majax]

Attachment: Disable permissions mochitests

Comment 101

[:gerard-majax]

Attachment: Turn mozFMRadio API to ChromeOnly r=fabrice sr=bz

Comment 102

[:gerard-majax]

Attachment: Turn mozTime API to ChromeOnly r=fabrice sr=bz

Comment 103

[:gerard-majax]

Attachment: Turn SystemUpdate API to ChromeOnly r=fabrice sr=bz

Comment 104

[:gerard-majax]

Attachment: Turn AudioChannelManager API to ChromeOnly r=fabrice sr=bz

Comment 105

[:gerard-majax]

Attachment: Turn mozChromeNotifications API to ChromeOnly r=fabrice sr=bz

Comment 106

[:gerard-majax]

Attachment: Switch mozChromeNotifications mochitests to chrome

Comment 107

[:gerard-majax]

Attachment: Turn EngineeringMode API to ChromeOnly r=fabrice sr=bz

Comment 108

[:gerard-majax]

Attachment: Turn mozTetheringManager API to ChromeOnly r=fabrice sr=bz

Comment 109

[:gerard-majax]

Attachment: Turn SimplePush API to ChromeOnly r=fabrice sr=bz

Comment 110

[:gerard-majax]

Attachment: th-logs.py

Tool to extract test execution informations from a try push

Comment 111

[:gerard-majax]

Attachment: try_without_bug1266035.json

result of try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b3b8c3a4d01

Comment 112

[:gerard-majax]

Attachment: try_with_bug1266035.json

result for try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b754f9af9ef

Comment 113

[:gerard-majax]

Attachment: th-logs.py

Comment 114

[:gerard-majax]

Attachment: try_without_bug1266035.json.bz2

Comment 115

[:gerard-majax]

Attachment: try_with_bug1266035.json.bz2

Comment 116

[:gerard-majax]

Attachment: try_bug1266035.diff

diff between try tests executions collected with th-logs.py

Comment 117

[:gerard-majax]

Attachment: try_bug1266035.diff

remove whitespace at the end of each line for easier reading

Comment 118

[:gerard-majax]

Attachment: try_bug1266035.diff

more context: more readable

Comment 119

[:gerard-majax]

Fabrice, Boris, sorry in advance for the spammy set of reviews. The WebIDL changes should be easy to review :)

Comment 120

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8752773 [details] [diff] [review]
Turn TV-related APIs to ChromeOnly r=fabrice sr=bz

sr=me

I'll be honest; I'm just rubber-stamping all these.  Not reading very carefully.

Comment 121

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8752797 [details] [diff] [review]
Turn UDP/TCP Socket APIs to ChromeOnly r=fabrice sr=bz

>+++ b/dom/network/TCPSocket.cpp

This change doesn't look right to me.  We used to expose TCPSocket in chrome, and if the pref was set also in places with the tcp-socket permission.  But now we're going to expose it in chrome, and also if the pref is set _everywhere_.

That doesn't seem right at all.

Comment 122

[:gerard-majax]

(In reply to Boris Zbarsky [:bz] from comment #121)
> Comment on attachment 8752797 [details] [diff] [review]
> Turn UDP/TCP Socket APIs to ChromeOnly r=fabrice sr=bz
> 
> >+++ b/dom/network/TCPSocket.cpp
> 
> This change doesn't look right to me.  We used to expose TCPSocket in
> chrome, and if the pref was set also in places with the tcp-socket
> permission.  But now we're going to expose it in chrome, and also if the
> pref is set _everywhere_.
> 
> That doesn't seem right at all.

I agree with you, but we are removing permissions also, so I don't see how we can limit the scope in chrome :/

Comment 123

[Boris Zbarsky [:bzbarsky]]

Well, what are the intended semanics of this pref?  Is the idea that this is exposed in chrome no matter the pref value and the pref value controls .... what?  Or is the idea that this is only exposed in chrome and only if the pref is set?

Comment 124

[:gerard-majax]

Given bug 1207090 I think we want to expose to any chrome, no? Maybe we can just kill the pref then?

Comment 125

[:gerard-majax]

Ok, given dom/webidl/TCPSocket.webidl we were only limiting the scope of mozTCPSocket object, but the TCPSocket itself was still exposed to content :/

Comment 126

[:gerard-majax]

Boris, as I said in comment 124, bug 1207090 enabled use of this whatever the pref value was. Since we are changing the scope to be limited to Chrome, I guess we can drop the |Func| part change the WebIDLs to be ChromeOnly, get rid of the pref?

Comment 127

[Boris Zbarsky [:bzbarsky]]

Seems about right to me, but check with jdm.

Comment 128

[:gerard-majax]

Josh, I need your confirmation regarding what is stated in comment 121, 124 and 126. Thanks!

Comment 129

[Josh Matthews [:jdm]]

It seems to me that making ShouldTCPSocketExist return false if it's called for a non-system principal would do the job.

Comment 130

[:gerard-majax]

Attachment: Turn UDP/TCP Socket APIs to ChromeOnly r=fabrice sr=bz

Comment 131

[:gerard-majax]

Attachment: Switch UDP/TCP Socket mochitests to chrome

Comment 132

[:gerard-majax]

Comment on attachment 8753556 [details] [diff] [review]
Turn UDP/TCP Socket APIs to ChromeOnly r=fabrice sr=bz

Updated the scoping so that:
 - it's not exposed to content
 - it's exposed to window chrome
 - it's exposed to jsm

Comment 133

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8753556 [details] [diff] [review]
Turn UDP/TCP Socket APIs to ChromeOnly r=fabrice sr=bz

I recommend replacing this pattern:

  if (foo) {
    return true;
  }

  return false;

with:

  return foo;

>+ Func="mozilla::dom::TCPSocket::ShouldTCPSocketExist", Exposed=(Window,System)]

Please keep this on two lines like it used to be.

Same in various other places.

sr=me

Comment 134

[:gerard-majax]

Attachment: Turn UDP/TCP Socket APIs to ChromeOnly r=fabrice sr=bz

Comment 135

[:gerard-majax]

Boris, is there something wrong with attachment 8752810 [details] [diff] [review] ? Or did it just slipped through because of the amount of reviews :) ?

Comment 136

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8752810 [details] [diff] [review]
Turn mozChromeNotifications API to ChromeOnly r=fabrice sr=bz

sr=me

Just got lost in the tab-storm.

Comment 137

[:gerard-majax]

Awesome, thanks!

Comment 138

[:gerard-majax]

https://hg.mozilla.org/projects/pine/pushloghtml?changeset=d3614e334308

Comment 139

[Josh Matthews [:jdm]]

Comment on attachment 8753720 [details] [diff] [review]
Turn UDP/TCP Socket APIs to ChromeOnly r=fabrice sr=bz

Review of attachment 8753720 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/network/TCPSocket.cpp
@@ -1222,5 @@
> -    return true;
> -  }
> -
> -  const char* const perms[] = { "tcp-socket", nullptr };
> -  return Preferences::GetBool("dom.mozTCPSocket.enabled") &&

Just to confirm, we're intentionally removing the TCP preference but leaving the UDP one?

Comment 140

[Josh Matthews [:jdm]]

Comment on attachment 8753559 [details] [diff] [review]
Switch UDP/TCP Socket mochitests to chrome

Review of attachment 8753559 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/network/tests/test_tcpsocket_client_and_server_basics.js
@@ -154,5 @@
>  }
>  
>  
>  function* test_basics() {
> -  if (enablePrefsAndPermissions()) {

Let's remove enablePrefsAndPermissions from tcpsocket_test.jsm and test_tcpsocket_client_and_server_basics.html.

::: dom/network/tests/test_tcpsocket_enabled_no_perm.html
@@ -15,5 @@
>  /** Test to ensure TCPSocket preference being turned on does not enable
>    navigator.mozTCPSocket.
>  **/
>  SimpleTest.waitForExplicitFinish();
>  SpecialPowers.pushPrefEnv({"set": [['dom.mozTCPSocket.enabled', true]]}, runTest);

If we don't use the preference anymore we shouldn't use it in the test.

::: dom/network/tests/test_tcpsocket_enabled_with_perm.html
@@ +16,5 @@
>    navigator.mozTCPSocket, and mozTCPSocket.open works when
>    the tcp-socket permission has been granted.
>  **/
>  SimpleTest.waitForExplicitFinish();
> +SpecialPowers.pushPrefEnv({"set": [['dom.mozTCPSocket.enabled', true]]}, runTest);

No need for this if we're not using the preference.

::: dom/network/tests/test_tcpsocket_legacy.html
@@ +18,5 @@
>  <pre id="test">
>  </pre>
>  <script>
>    SimpleTest.waitForExplicitFinish();
>    SpecialPowers.pushPrefEnv(

No need for this if we're not using the preference.

Comment 141

[Boris Zbarsky [:bzbarsky]]

This landed on pine 2 weeks ago.  When is it going to merge to m-c?  The review for the codegen was obviously conditional on this landing on m-c.

Having this stuff still around on m-c is going to start causing problems in about 5 minutes, because I will be making codegen changes that will conflict with these...

Comment 142

[:gerard-majax]

(In reply to Boris Zbarsky [:bz] from comment #141)
> This landed on pine 2 weeks ago.  When is it going to merge to m-c?  The
> review for the codegen was obviously conditional on this landing on m-c.
> 
> Having this stuff still around on m-c is going to start causing problems in
> about 5 minutes, because I will be making codegen changes that will conflict
> with these...

Yes, it is planned to merge as soon as possible, but there are still reftests on pine that are broken. This is all pending reviews mostly.

Comment 143

[:gerard-majax]

FYI there has already been several cases of m-c => pine conflicts around since we landed and merged over the days, I don't think it's such a big deal to have some new ones :)

Comment 144

[Ryan VanderMeulen (PTO, back July 27)]

Is there a bug on file for fixing and re-enabling all the tests in dom/browser-element/mochitest this disabled?

Comment 145

[:gerard-majax]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #144)
> Is there a bug on file for fixing and re-enabling all the tests in
> dom/browser-element/mochitest this disabled?

I don't think so