814556 - B2G RIL: clean up scattered MOZ_B2G_RIL preprocessor in dom/base/Navigator.cpp

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Vicamo Yang [:vicamo][:vyang]]

See bug 778093 comment #102.

Comment 1

[Vicamo Yang [:vicamo][:vyang]]

See bug 778093 comment #138, should also clean-up installed packages definitions in all applications.

Comment 2

[Vicamo Yang [:vicamo][:vyang]]

Nominate for koi+ for blocking bug 920551, which is koi+.

Comment 3

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 1/3: Move mozilla::dom::icc::* to mozilla::dom

Like all other RIL components, move DOM components into mozilla::dom.  Then we can move forward declaration of mozilla::dom::IccManager the line after CellBroadcast in Navigator.h

Comment 4

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 2/3: Don't guard MozTelephony with MOZ_B2G_RIL.

WebTelephony has been fully converted to use WebIDL.  From bug 859616 comment 13, we're going to hide features with WebIDL.  For WebTelephony, this is done in bug 915604.

Comment 5

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 3/3: clean up {b2g,browser}/installer/package-manifest.in

1. Remove all MOZ_B2G_RIL guarded components from browser/installer/package-manifest.in
2. MobileMessage is built regardless MOZ_B2G_RIL.  The only difference is it installs MmsService and MobileMessageDatabaseService when applied to MOZ_WIDGET_TOOLKIT == 'gonk'

Comment 6

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 2/3: Don't guard MozTelephony with MOZ_B2G_RIL : v2

fix b2g-desktop build bustage on Linux

Comment 7

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 3/3: clean up {b2g,browser}/installer/package-manifest.in : v2

fix b2g-desktop build bustage on Linux

Comment 8

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 2/3: Don't guard MozTelephony with MOZ_B2G_RIL : v3

fix build bustage

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

Breaks all Gaia UI tests - https://tbpl.mozilla.org/?tree=Try&rev=8bf7258ffff1

In gaia/apps/system/js/lockscreen.js:

  190     /* Telephony changes */
  191     if (navigator.mozTelephony) {
  192       navigator.mozTelephony.addEventListener('callschanged', this);
  193     }

Because |navigator.mozTelephony| creation process tries to talk to backend immediately for call enumeration and b2g-desktop has no WebTelephony backend, it fails and throws an error that blocks further Gaia UI test process.

Comment 10

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 1/3: Move mozilla::dom::icc::* to mozilla::dom : v2

Eliminate LOCAL_INCLUDES path for dom/icc/src in other components as well.

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 3/3: clean up {b2g,browser}/installer/package-manifest.in : v3

Rebase only.

Comment 12

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 1/2: Move mozilla::dom::icc::* to mozilla::dom : v3

I'm going to obsolete original part 2/3, which breaks Gaia UI tests.  So it makes this patch part 1/2.

Comment 13

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 2/2: clean up MOZ_B2G_RIL in browser/installer/package-manifest.in and dom/base/Navigator.* : v4

1) Obsolete original part 2/3 because it breaks Gaia UI test.
2) Don't touch MOZ_B2G_RIL & MOZ_WIDGET_GONK correction.  Let bug 920551 deal with them instead.

Try: https://tbpl.mozilla.org/?tree=Try&rev=14fe2ae17415

Comment 14

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

Comment on attachment 819489 [details] [diff] [review]
part 1/2: Move mozilla::dom::icc::* to mozilla::dom : v3

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

Seems sensible enough

::: dom/icc/src/StkCommandEvent.cpp
@@ -11,3 @@
>  
> -namespace mozilla {
> -namespace dom {

I prefer wrapping the file in namespaces over 'using'.

::: dom/icc/src/StkCommandEvent.h
@@ +1,5 @@
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this file,
>   * You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +#ifndef mozilla_dom_stkcommandevent_h

Nit: mixed case StkCommandEvent

Comment 15

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

Comment on attachment 819491 [details] [diff] [review]
part 2/2: clean up MOZ_B2G_RIL in browser/installer/package-manifest.in and dom/base/Navigator.* : v4

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

> Bug 814556 - 2/2: clean up MOZ_B2G_RIL in b2g/installer/package-manifest.in and dom/base/Navigator.*. r=khuey

You actually changed browser/installer/package-manifest.in.

No idea about package-manifest.in, Navigator.* changes seem fine.

Comment 16

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 819489 [details] [diff] [review]
part 1/2: Move mozilla::dom::icc::* to mozilla::dom : v3

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

::: dom/icc/src/StkCommandEvent.cpp
@@ -11,3 @@
>  
> -namespace mozilla {
> -namespace dom {

I don't care either way.

::: dom/icc/src/StkCommandEvent.h
@@ +1,5 @@
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this file,
>   * You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +#ifndef mozilla_dom_stkcommandevent_h

But please fix this (and the bottom of the file too).

Comment 17

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 819491 [details] [diff] [review]
part 2/2: clean up MOZ_B2G_RIL in browser/installer/package-manifest.in and dom/base/Navigator.* : v4

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

Why are you removing this stuff from the package-manifest.in?

Comment 18

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #17)
> part 2/2: clean up MOZ_B2G_RIL in browser/installer/package-manifest.in and
> dom/base/Navigator.* : v4
> 
> Review of attachment 819491 [details] [diff] [review]:
> -----------------------------------------------------------------
> Why are you removing this stuff from the package-manifest.in?

From bug 778093 comment 138, we really don't have RIL support on desktop builds.  For b2g-desktop, we still use 'b2g' as application name, so basically these entries are not meaningful in any way.

Comment 19

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 819491 [details] [diff] [review]
part 2/2: clean up MOZ_B2G_RIL in browser/installer/package-manifest.in and dom/base/Navigator.* : v4

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

Ok.  r=me

Comment 20

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 1/2: Move mozilla::dom::icc::* to mozilla::dom : v4

Address comment 14, wrap code with namespace statements and correct inclusion guards names to reflect header file name.

Comment 21

[Vicamo Yang [:vicamo][:vyang]]

Attachment: part 2/2: clean up MOZ_B2G_RIL in browser/installer/package-manifest.in and dom/base/Navigator.* : v5

Address comment 15, correct path in commit summary.

Comment 22

[Vicamo Yang [:vicamo][:vyang]]

Will push when Gu test turns green on b2g-inbound.

Comment 23

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/b2g-inbound/rev/db80d2993a26
https://hg.mozilla.org/integration/b2g-inbound/rev/666ca6ffb5ef

Comment 24

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/db80d2993a26
https://hg.mozilla.org/mozilla-central/rev/666ca6ffb5ef

Comment 25

[Preeti Raghunath(:Preeti)]

koi+ for flatfish

Comment 26

[Ed Morley [:emorley]]

This will need an updated patch for aurora/koi uplift:

[/c/src-gecko/aurora]$ transplant db80d2993a26
searching for changes
applying db80d2993a26
patching file dom/bindings/Bindings.conf
Hunk #2 FAILED at 1865
1 out of 2 hunks FAILED -- saving rejects to file dom/bindings/Bindings.conf.rej
patching file dom/bindings/Makefile.in
Hunk #1 FAILED at 89
1 out of 1 hunks FAILED -- saving rejects to file dom/bindings/Makefile.in.rej
patching file dom/icc/src/IccManager.cpp
Hunk #1 FAILED at 0
1 out of 3 hunks FAILED -- saving rejects to file dom/icc/src/IccManager.cpp.rej
patch failed to apply

Comment 27

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Ed Morley [:edmorley UTC+1] from comment #26)
> This will need an updated patch for aurora/koi uplift:

Working on it :)

Comment 28

[Vicamo Yang [:vicamo][:vyang]]

Attachment: [aurora] part 1/2

Comment 29

[Vicamo Yang [:vicamo][:vyang]]

Attachment: [aurora] part 2/2

Comment 30

[Vicamo Yang [:vicamo][:vyang]]

try before landing: https://tbpl.mozilla.org/?tree=Try&rev=33f6aef13c56

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1a9770998e84
https://hg.mozilla.org/releases/mozilla-aurora/rev/561e09c7b506