1236433 - [OTA][RIL] Separate Gecko and Gonk layers for accessing Androids properties

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Use this bug to address the solution to the RIL module.

Comment 1

[Bevis Tseng[:bevis][:btseng](Exited)]

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

Comment 2

[Tim Huang[:timhuang]]

Would you provide the js counterpart as well?

Comment 3

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Hi

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #1)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fc8c41ff266

Two comments on the patch set:

 1) The wrapper isn't actually DOM code. Shouldn't it be located in hal/? 

 2) We should think about an asynchronous interface: the Android property functions can block AFAICT, and other systems might require communication with an external process for maintaining properties (e.g, dconf on Linux).

Comment 4

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Tim Huang[:timhuang] from comment #2)
> Would you provide the js counterpart as well?

The JS one is already available in 
https://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/systemlibs.js?case=true&from=systemlibs.js

Comment 5

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #3)
> Hi
Hi!
> (In reply to Bevis Tseng[:bevistseng][:btseng] from comment #1)
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fc8c41ff266
> 
> Two comments on the patch set:
> 
>  1) The wrapper isn't actually DOM code. Shouldn't it be located in hal/? 
> 
m... No, this interface is still a gonk-specific one instead of a generic one to be defined in hal/. and is the counterpart of the JS one in dom/system/gonk/systemlibs.js.
By my understanding, the purpose of having interfaces in hal/ is to provide generic interfaces on top of some hardware/system-level features in different platforms. 
I didn't see the purpose of these APIs on different platforms yet.
>  2) We should think about an asynchronous interface: the Android property
> functions can block AFAICT, and other systems might require communication
> with an external process for maintaining properties (e.g, dconf on Linux).
It seems that there are 2 goals you would like achieve:
1. Async interface for android property.
2. Define a generic property interface that could be used in different platform.

However, the problem I'd like to solve in this bug is to prevent any linkage from gonk clients inside gecko to the android library directly in build time and provide a dummy fallback if not available which allows us to update gecko without the dependency to the underlying version of gecko.

For Goal#1, there will be much impact on current design because lots of access to system property are done at the initialization-stage of the modules and the use case of the API of these modules are in synchronized way, like:
>    nsCOMPtr<nsIXXXService> service =
>      do_GetService(NS_XXX_SERVICE_CONTRACTID);
>    nsresult rv = service->GetXXXX();
All these API design has to be changed, before adopting the async approach. :(
We need another meta bug to discuss this if async design is required.

For Goal#2, I didn't see proper use cases yet for this API in other platform.
Hence, the purpose of making this gonk-specific API to a generic one is still uncertain.
In addition, IMHO, the alternative solution for Goal#2 could be a mechanism to map the preferences in different platform to the SharedPreferences that already available and used in gecko.

Comment 6

[Bevis Tseng[:bevis][:btseng](Exited)]

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

Comment 7

[Bevis Tseng[:bevis][:btseng](Exited)]

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

Comment 8

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: MozReview Request: Bug 1236433 - Part 1: Provide a Native Wrapper to Allow Fallback When property_get/set is Unavailable; r=edgar

Review commit: https://reviewboard.mozilla.org/r/30283/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30283/

Comment 9

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: MozReview Request: Bug 1236433 - Part 2: Adopt Wrapper in Network Utilities; r=edgar

Review commit: https://reviewboard.mozilla.org/r/30285/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30285/

Comment 10

[Edgar Chen [:edgar]]

Comment on attachment 8706200 [details]
MozReview Request: Bug 1236433 - Part 1: Provide a Native Wrapper to Allow Fallback When property_get/set is Unavailable; r=edgar

https://reviewboard.mozilla.org/r/30283/#review27457

Comment 11

[Edgar Chen [:edgar]]

Comment on attachment 8706201 [details]
MozReview Request: Bug 1236433 - Part 2: Adopt Wrapper in Network Utilities; r=edgar

https://reviewboard.mozilla.org/r/30285/#review27459

Looks good, thank you.

Comment 12

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8706201 [details]
MozReview Request: Bug 1236433 - Part 2: Adopt Wrapper in Network Utilities; r=edgar

https://reviewboard.mozilla.org/r/30285/#review27461

Comment 13

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8706200 [details]
MozReview Request: Bug 1236433 - Part 1: Provide a Native Wrapper to Allow Fallback When property_get/set is Unavailable; r=edgar

https://reviewboard.mozilla.org/r/30283/#review27463

Comment 14

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8706200 [details]
MozReview Request: Bug 1236433 - Part 1: Provide a Native Wrapper to Allow Fallback When property_get/set is Unavailable; r=edgar

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30283/diff/1-2/

Comment 15

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8706201 [details]
MozReview Request: Bug 1236433 - Part 2: Adopt Wrapper in Network Utilities; r=edgar

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30285/diff/1-2/

Comment 16

[Bevis Tseng[:bevis][:btseng](Exited)]

See try server result in comment 7.

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/6d668b0ef54f
https://hg.mozilla.org/integration/b2g-inbound/rev/b51f4ee1b2f5

Comment 18

[(Out Sep 17-23) Ryan VanderMeulen]

https://hg.mozilla.org/mozilla-central/rev/6d668b0ef54f
https://hg.mozilla.org/mozilla-central/rev/b51f4ee1b2f5