Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Skip to content
Commit c03c9774 authored by Neil Fuller's avatar Neil Fuller
Browse files

Remove some usages of NitzData.isDst()

TL;DR - removes some usages of NitzData.isDst() because it's a bad
code smell. The changes should mostly not affect time zone detection for
real users. This commit also improves documentation for the NitzData
class.

Remove some usages of NitzData.isDst() because it conflates two states:
1) DST state is not known.
2) DST state is known to be false.

Generally, this conflation of two states is not useful but it dates from
a time when telephony code couldn't tell the difference: the DST offset
used to be a primitive int, when DST info wasn't known it was left as
zero, leading to ambiguity. The NitzData.isDst() method was provided to
avoid changing too much behavior when the telephony code was rewritten.

The codepaths being changed are used in the following circumstances:

1) NitzStateMachineImpl.isNitzSignalOffsetInfoBogus() - determines
whether there is a good chance that the NitzData signal for a given
country is bogus/erroneous (i.e. just the result of unset placeholder /
zero values) and ignores it if it is. It does this by looking at
the NitzData to see if it could be using unintialized (zero) values
(potentially in lower levels of the stack). If bad values look possible,
it looks at the current country's time zones to see if UTC / no DST is a
possibility for the country. This logic is primarily an optimization to
avoid fruitlessly searching for UTC zones within countries that never
use UTC.

This commit removes DST state from the NITZ bogus data check and now
only looks at the total offset. It is unclear if a bogus NITZ signal
would include no DST information at all or the device would receive a
zero DST offset value. Checking the DST looks like a "bonus" check and
should be safe to remove.

2) TimeZoneLookupHelper.lookupByNitzStatic() - tries to find a matching
zone using NitzData only. This is only expected to happen in test
scenarios, i.e. when the country is "known" but there is no country code
because the MCC is not recognized and something like 001. In test
scenarios there is no expectation of a "good" match, just a match of a
zone that has the correct offset.

Previously the code would use the isDst() boolean value then try the
inverse if that didn't match. Now, it will properly handle when the DST
information is not known (i.e. it will only try to match once) and it
will still try a match without DST matching if the DST match didn't
succeed.

This method has also been cleaned up to avoid the assumption that DST is
always one hour ahead, which is generally true but not guaranteed. This
involves changing from TimeZone.getAvailableIDs(int) to
TimeZone.getAvailableIDs(). Inspection of the ICU code revealed that
getAvailableIDs(int) is inefficient and involves instantiating every
time zone known to inspect the raw offset. ICU has some soft reference
caching but that instantiation might as well be put off until the
matching takes place as this will avoid ICU instanting / caching
TimeZone objects for every known ID unnecessarily in many cases.  Since
the code is only intended for use in test scenarios this clean up is
mostly about simplification.

Bug: 140712361
Test: treehugger only
Change-Id: Ic9cef36627ce7a33c2da6afdff14b52a5d0b7c4c
parent ab9f1d6b
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment