Closed Bug 814556 Opened 7 years ago Closed 6 years ago

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

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:koi+, firefox25 wontfix, firefox26 fixed, firefox27 fixed, b2g-v1.2 fixed)

RESOLVED FIXED
1.2 C3(Oct25)
blocking-b2g koi+
Tracking Status
firefox25 --- wontfix
firefox26 --- fixed
firefox27 --- fixed
b2g-v1.2 --- fixed

People

(Reporter: vicamo, Assigned: vicamo)

References

Details

(Whiteboard: flatfish only)

Attachments

(4 files, 10 obsolete files)

13.20 KB, patch
Details | Diff | Splinter Review
6.40 KB, patch
Details | Diff | Splinter Review
11.95 KB, patch
Details | Diff | Splinter Review
6.41 KB, patch
Details | Diff | Splinter Review
See bug 778093 comment #138, should also clean-up installed packages definitions in all applications.
Component: DOM: Device Interfaces → RIL
Product: Core → Boot2Gecko
Blocks: 920551
Nominate for koi+ for blocking bug 920551, which is koi+.
blocking-b2g: --- → koi?
Target Milestone: --- → 1.2 C3(Oct25)
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
Assignee: nobody → vyang
Status: NEW → ASSIGNED
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.
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'
fix b2g-desktop build bustage on Linux
Attachment #818640 - Attachment is obsolete: true
fix b2g-desktop build bustage on Linux
Attachment #818658 - Attachment is obsolete: true
fix build bustage
Attachment #818723 - Attachment is obsolete: true
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.
Depends on: 928232
Attachment #818634 - Flags: review?(khuey)
Attachment #818634 - Flags: feedback?(VYV03354)
Attachment #818634 - Flags: feedback?(Ms2ger)
Attachment #818724 - Flags: review?(khuey)
Attachment #818724 - Flags: feedback?(VYV03354)
Attachment #818724 - Flags: feedback?(Ms2ger)
Attachment #818818 - Flags: review?(khuey)
Attachment #818818 - Flags: feedback?(VYV03354)
Attachment #818818 - Flags: feedback?(Ms2ger)
Eliminate LOCAL_INCLUDES path for dom/icc/src in other components as well.
Attachment #818634 - Attachment is obsolete: true
Attachment #818634 - Flags: review?(khuey)
Attachment #818634 - Flags: feedback?(VYV03354)
Attachment #818634 - Flags: feedback?(Ms2ger)
Attachment #819305 - Flags: review?(khuey)
Attachment #819305 - Flags: feedback?(VYV03354)
Attachment #819305 - Flags: feedback?(Ms2ger)
Rebase only.
Attachment #818724 - Attachment is obsolete: true
Attachment #818724 - Flags: review?(khuey)
Attachment #818724 - Flags: feedback?(VYV03354)
Attachment #818724 - Flags: feedback?(Ms2ger)
Attachment #819306 - Flags: review?(khuey)
Attachment #819306 - Flags: feedback?(VYV03354)
Attachment #819306 - Flags: feedback?(Ms2ger)
No longer depends on: 928232
I'm going to obsolete original part 2/3, which breaks Gaia UI tests.  So it makes this patch part 1/2.
Attachment #819305 - Attachment is obsolete: true
Attachment #819305 - Flags: review?(khuey)
Attachment #819305 - Flags: feedback?(VYV03354)
Attachment #819305 - Flags: feedback?(Ms2ger)
Attachment #819489 - Flags: review?(khuey)
Attachment #819489 - Flags: feedback?(VYV03354)
Attachment #819489 - Flags: feedback?(Ms2ger)
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
Attachment #818818 - Attachment is obsolete: true
Attachment #819306 - Attachment is obsolete: true
Attachment #818818 - Flags: review?(khuey)
Attachment #818818 - Flags: feedback?(VYV03354)
Attachment #818818 - Flags: feedback?(Ms2ger)
Attachment #819306 - Flags: review?(khuey)
Attachment #819306 - Flags: feedback?(VYV03354)
Attachment #819306 - Flags: feedback?(Ms2ger)
Attachment #819491 - Flags: review?(khuey)
Attachment #819491 - Flags: feedback?(VYV03354)
Attachment #819491 - Flags: feedback?(Ms2ger)
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
Attachment #819489 - Flags: feedback?(Ms2ger) → feedback+
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.
Attachment #819491 - Flags: feedback?(Ms2ger) → feedback+
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).
Attachment #819489 - Flags: review?(khuey) → review+
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?
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) 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.
Flags: needinfo?(vyang)
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
Attachment #819491 - Flags: review?(khuey) → review+
Address comment 14, wrap code with namespace statements and correct inclusion guards names to reflect header file name.
Attachment #819489 - Attachment is obsolete: true
Attachment #819489 - Flags: feedback?(VYV03354)
Address comment 15, correct path in commit summary.
Attachment #819491 - Attachment is obsolete: true
Attachment #819491 - Flags: feedback?(VYV03354)
Will push when Gu test turns green on b2g-inbound.
koi+ for flatfish
blocking-b2g: koi? → koi+
Whiteboard: flatfish only
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
Whiteboard: flatfish only → flatfish only [needs-aurora-patch]
(In reply to Ed Morley [:edmorley UTC+1] from comment #26)
> This will need an updated patch for aurora/koi uplift:

Working on it :)
Attachment #822779 - Attachment description: [b2g26_v1_2] part 1/2 → [aurora] part 1/2
Attachment #822780 - Attachment description: [b2g26_v1_2] part 2/2 → [aurora] part 2/2
try before landing: https://tbpl.mozilla.org/?tree=Try&rev=33f6aef13c56
Whiteboard: flatfish only [needs-aurora-patch] → flatfish only
You need to log in before you can comment on or make changes to this bug.