Remove MOZ_WIDGET_GONK completely

RESOLVED FIXED in Firefox 56

Status

()

Core
General
RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: njn, Assigned: njn)

Tracking

(Blocks: 1 bug)

54 Branch
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(12 attachments, 2 obsolete attachments)

19.15 KB, patch
erahm
: review+
njn
: checkin+
Details | Diff | Splinter Review
19.87 KB, patch
glandium
: review+
njn
: checkin+
Details | Diff | Splinter Review
3.48 KB, patch
glandium
: review+
njn
: checkin+
Details | Diff | Splinter Review
10.29 KB, patch
jld
: review+
njn
: checkin+
Details | Diff | Splinter Review
2.02 KB, patch
glandium
: review+
njn
: checkin+
Details | Diff | Splinter Review
9.38 KB, patch
kats
: review+
njn
: checkin+
Details | Diff | Splinter Review
7.90 KB, patch
jld
: review+
njn
: checkin+
Details | Diff | Splinter Review
19.28 KB, patch
mccr8
: review+
njn
: checkin+
Details | Diff | Splinter Review
24.61 KB, patch
gerald
: review+
njn
: checkin+
Details | Diff | Splinter Review
26.75 KB, patch
mccr8
: review+
jdm
: review+
njn
: checkin+
Details | Diff | Splinter Review
7.21 KB, patch
snorp
: review+
njn
: checkin+
Details | Diff | Splinter Review
3.90 KB, patch
gsvelto
: review+
njn
: checkin+
Details | Diff | Splinter Review
(Assignee)

Description

10 months ago
Gonk widget support is dead.

- bug 1355752 comment 25 is where the decision was made to stop supporting it.

- Bug 1357323 removed support for building the Gonk widget code, as well as dom/system/gonk/, hal/gonk/, and some other Gonk-specific code.

- Bug 1373478 removed widget/gonk/.

This bug is about removing all remaining traces of MOZ_WIDGET_GONK.
(Assignee)

Comment 1

10 months ago
Created attachment 8887765 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from xpcom/

As well as the straightforward things, this lets us remove ReadSysFile and
WriteSysFile, which in turn lets us remove TestFileUtils.cpp.
Attachment #8887765 - Flags: review?(erahm)
(Assignee)

Updated

10 months ago
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
(Assignee)

Comment 2

10 months ago
Created attachment 8887768 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from toolkit/
Attachment #8887768 - Flags: review?(mh+mozilla)
(Assignee)

Comment 3

10 months ago
Created attachment 8887770 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from mozglue/
Attachment #8887770 - Flags: review?(mh+mozilla)
(Assignee)

Comment 4

10 months ago
Created attachment 8887781 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from security/
Attachment #8887781 - Flags: review?(jld)
(Assignee)

Comment 5

10 months ago
Created attachment 8887786 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from memory/
Attachment #8887786 - Flags: review?(mh+mozilla)
(Assignee)

Comment 6

10 months ago
Created attachment 8887789 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from modules/libpref/init/all.js
Attachment #8887789 - Flags: review?(gsvelto)
(Assignee)

Comment 7

10 months ago
Created attachment 8887791 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from modules/libpref/init/all.js
Attachment #8887791 - Flags: review?(gsvelto)
(Assignee)

Updated

10 months ago
Attachment #8887789 - Attachment is obsolete: true
Attachment #8887789 - Flags: review?(gsvelto)
(Assignee)

Comment 8

10 months ago
Created attachment 8887795 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from ipc/
Attachment #8887795 - Flags: review?(jld)

Updated

10 months ago
Attachment #8887768 - Flags: review?(mh+mozilla) → review+

Updated

10 months ago
Attachment #8887770 - Flags: review?(mh+mozilla) → review+

Updated

10 months ago
Attachment #8887786 - Flags: review?(mh+mozilla) → review+
See Also: → bug 1382246
Comment on attachment 8887765 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from xpcom/

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

Nice cleanup, thanks!
Attachment #8887765 - Flags: review?(erahm) → review+
Comment on attachment 8887781 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from security/

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

r=me.  A couple comments, but no need to change anything.

::: security/manager/pki/nsNSSDialogHelper.cpp
@@ -22,5 @@
> -#ifdef MOZ_WIDGET_GONK
> -  // On b2g devices, we need to proxy the dialog creation & management
> -  // to Gaia.
> -  return NS_ERROR_NOT_IMPLEMENTED;
> -#endif

I'm not officially a peer for this, but it seems reasonable.

::: security/sandbox/linux/broker/SandboxBroker.cpp
@@ -459,5 @@
> -  static const long nr_setreuid = __NR_setreuid;
> -  static const long nr_setregid = __NR_setregid;
> -#endif
> -  if (syscall(nr_setregid, getgid(), AID_APP + mChildPid) != 0 ||
> -      syscall(nr_setreuid, getuid(), AID_APP + mChildPid) != 0) {

Thanks for reminding me about this.  I filed bug 1382246 for some related cleanup.

::: security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp
@@ +39,5 @@
>  SandboxBrokerPolicyFactory::SandboxBrokerPolicyFactory()
>  {
>    // Policy entries that are the same in every process go here, and
>    // are cached over the lifetime of the factory.
> +#if defined(MOZ_CONTENT_SANDBOX)

FYI, this part is likely to have merge conflicts with bug 1308400.
Attachment #8887781 - Flags: review?(jld) → review+
Comment on attachment 8887795 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from ipc/

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

r=me.  Again, there are some additional removals that can happen, but those can happen in bug 1316153; this patch is fine as-is.

::: ipc/chromium/src/base/process_util_linux.cc
@@ +26,3 @@
>  /*
> + * We fall back to an arbitrary UID. This is generally the UID for user
> + * `nobody', albeit it is not always the case.

As far as I know we can rip out *all* the stuff about uids/gids here; B2G was the only platform where it could be used.  (I think the “platforms that are not gonk based” comment was a reference to porting B2G to a non-Android-based Linux, like what bug 731498 proposed.)

Bug 1316153 already covers this, so I'll comment over there to explicitly mention it.

::: ipc/glue/GeckoChildProcessHost.cpp
@@ +76,5 @@
>  #endif
>  
> +// We currently don't drop privileges on any platform, because we have to worry
> +// about plugins and extensions breaking.
> +static const bool kLowRightsSubprocesses = false;

This can be constant-propagated, but that cleanup is also part of bug 1316153.
Attachment #8887795 - Flags: review?(jld) → review+
(Assignee)

Updated

10 months ago
Attachment #8887768 - Flags: checkin+
(Assignee)

Updated

10 months ago
Attachment #8887770 - Flags: checkin+
(Assignee)

Updated

10 months ago
Attachment #8887786 - Flags: checkin+
(Assignee)

Comment 13

10 months ago
Created attachment 8888119 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from dom/{base,ipc,plugins}
Attachment #8888119 - Flags: review?(continuation)
(Assignee)

Comment 14

10 months ago
Created attachment 8888149 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from dom/media/

As well as the obvious #ifdefs, this allows DOMHwMediaStream to be
removed, and also the "phone-state-changed" observer.
Attachment #8888149 - Flags: review?(gsquelart)
(Assignee)

Comment 15

10 months ago
Created attachment 8888163 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from several dom/ subdirectories

As well as the obvious #ifdef stuff, the patch removes
TCPSocket::SetAppIdAndBrowser(), which means
{TCPSocketParent,TCPServerSocketParent}::{GetAppId,GetInIsolatedMozBrowser}()
can also be removed.
Attachment #8888163 - Flags: review?(continuation)
(Assignee)

Comment 16

10 months ago
Created attachment 8888171 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from media/, uriloader/, widget, /xpfe/
Attachment #8888171 - Flags: review?(snorp)
(Assignee)

Comment 17

10 months ago
Created attachment 8888172 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from media/, uriloader/, widget, /xpfe/
Attachment #8888172 - Flags: review?(snorp)
(Assignee)

Updated

10 months ago
Attachment #8888171 - Attachment is obsolete: true
Attachment #8888171 - Flags: review?(snorp)
Comment on attachment 8888119 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from dom/{base,ipc,plugins}

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

I'm not a peer of any of this code, but the patch is pretty trivial.
Attachment #8888119 - Flags: review?(continuation) → review+
https://hg.mozilla.org/mozilla-central/rev/932057f7c805
https://hg.mozilla.org/mozilla-central/rev/bd8cce1617e8
https://hg.mozilla.org/mozilla-central/rev/d11f37fe35f7
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
(Assignee)

Comment 20

10 months ago
More patches to land.
Status: RESOLVED → REOPENED
Keywords: leave-open
Resolution: FIXED → ---

Updated

10 months ago
Duplicate of this bug: 1382694
No longer depends on: 1382694
Attachment #8887791 - Flags: review?(gsvelto) → review+
Comment on attachment 8888163 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from several dom/ subdirectories

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

I didn't review the dom/network changes, as I don't know anything about that code and it was more complex than deleting some #ifdef blocks. Maybe jdm could review it.

::: dom/html/nsIFormControl.h
@@ +279,1 @@
>           // On Android/B2G, date/time input appears as a normal text box.

Drop the reference to B2G in the comment while you are here, maybe?
Attachment #8888163 - Flags: review?(josh)
Attachment #8888163 - Flags: review?(continuation)
Attachment #8888163 - Flags: review+

Updated

10 months ago
Attachment #8888163 - Flags: review?(josh) → review+
(Assignee)

Comment 23

10 months ago
Created attachment 8888585 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from hal/
Attachment #8888585 - Flags: review?(gsvelto)
(Assignee)

Updated

10 months ago
Attachment #8887781 - Flags: checkin+
(Assignee)

Updated

10 months ago
Attachment #8887791 - Flags: checkin+
(Assignee)

Updated

10 months ago
Attachment #8887795 - Flags: checkin+
(Assignee)

Updated

10 months ago
Attachment #8888119 - Flags: checkin+
(Assignee)

Updated

10 months ago
Attachment #8888163 - Flags: checkin+
(Assignee)

Updated

10 months ago
Attachment #8887765 - Flags: checkin+
Comment on attachment 8888585 [details] [diff] [review]
Remove MOZ_WIDGET_GONK from hal/

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

LGTM but AFAIK most of the power-related methods were only used in MozPowerManager which I doubt we're using on any other platform. From the looks of it it's still living somewhere within the Navigator object:

https://dxr.mozilla.org/mozilla-central/rev/1b065ffd8a535a0ad4c39a912af18e948e6a42c1/dom/base/Navigator.cpp#1428

We should probably file a bug to remove it.
Attachment #8888585 - Flags: review?(gsvelto) → review+
(Assignee)

Comment 26

10 months ago
> We should probably file a bug to remove it.

I filed bug 1382955 and made an attempt at a patch.
Attachment #8888172 - Flags: review?(snorp) → review+

Updated

10 months ago
Attachment #8888149 - Flags: review?(gsquelart) → review+
(Assignee)

Updated

10 months ago
Attachment #8888149 - Flags: checkin+
(Assignee)

Updated

10 months ago
Attachment #8888172 - Flags: checkin+
(Assignee)

Updated

10 months ago
Attachment #8888585 - Flags: checkin+
(Assignee)

Updated

10 months ago
Keywords: leave-open

Comment 29

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c9b593e3e697
https://hg.mozilla.org/mozilla-central/rev/f645062a665f
https://hg.mozilla.org/mozilla-central/rev/24df0c0bf28d
Status: REOPENED → RESOLVED
Last Resolved: 10 months ago10 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.