Make callers of FromUTC(Local)Exploded in chrome/ use new time API.

Make callers of FromUTC(Local)Exploded in chrome/ use new time API.

Use new time conversion API in accordance with
https://codereview.chromium.org/1988663002/

BUG=601900
Committed: https://crrev.com/b4dbc73df4c9c1c9bc3439d4c0f79c79b64f18f4
Cr-Commit-Position: refs/heads/master@{#404106}

[maksims (do not use this acc), 2016-07-04 11:21:40]

Description was changed from

==========
chrome

BUG=
==========

to

==========
Make callers of FromUTC(Local)Exploded in chrome/ use new time API. 

Use new time conversion API in accordance with 
https://codereview.chromium.org/1988663002/

BUG=601905
==========

[maksims (do not use this acc), 2016-07-04 11:25:39]

maksim.sisov@intel.com changed reviewers:
+ benwells@chromium.org, jam@chromium.org, jyquinn@chromium.org,
thestig@chromium.org, tsergeant@chromium.org

[maksims (do not use this acc), 2016-07-04 11:26:12]

Description was changed from

==========
Make callers of FromUTC(Local)Exploded in chrome/ use new time API. 

Use new time conversion API in accordance with 
https://codereview.chromium.org/1988663002/

BUG=601905
==========

to

==========
Make callers of FromUTC(Local)Exploded in chrome/ use new time API. 

Use new time conversion API in accordance with 
https://codereview.chromium.org/1988663002/

BUG=601900
==========

[maksims (do not use this acc), 2016-07-04 12:02:15]

Patchset #1 (id:1) has been deleted

[maksims (do not use this acc), 2016-07-05 09:59:43]

Patchset #1 (id:20001) has been deleted

[maksims (do not use this acc), 2016-07-05 10:00:00]

please review

[vabr (Chromium), 2016-07-05 10:59:53]

vabr@chromium.org changed reviewers:
+ vabr@chromium.org

[vabr (Chromium), 2016-07-05 10:59:54]

Hi maksims@,

When adding multiple reviewers, please specify who should have a look at which
file.


Having said that, your change in
chrome/browser/password_manager/password_store_mac.cc LGTM if you address my
comment below.

Cheers,
Vaclav

https://codereview.chromium.org/2111103002/diff/40001/chrome/browser/password...
File chrome/browser/password_manager/password_store_mac.cc (right):

https://codereview.chromium.org/2111103002/diff/40001/chrome/browser/password...
chrome/browser/password_manager/password_store_mac.cc:282: // time_string_bytes
was parsable. If the return value is false, the value of
The last sentence of this comment ("If the return ... is unchanged.") is no
longer true, please remove it. I checked the (only) callsite and it does not
seem to rely on this promise, so just dropping the comment is OK.

[Jackie Quinn, 2016-07-05 11:21:42]

ios lgtm

[tsergeant, 2016-07-05 23:16:23]

browsing_history_handler.cc lgtm

[benwells, 2016-07-06 00:24:52]

c/b/banners
c/b/engagement
c/b/extensions all lgtm

As mentioned already, please tell people what they want them to review when
there are multiple reviewers.

[maksims (do not use this acc), 2016-07-06 09:56:49]

Lei Zhang and jam, would you please review the following in accordance to
ownerships:
chrome/browser/chromeos/  	
chrome/browser/media_galleries 
chrome/browser/ui/webui/media/webrtc_logs_ui.cc  
chrome/installer/gcapi/gcapi_omaha_experiment.cc  
chrome/utility/media_galleries/picasa_album_table_reader.cc

https://codereview.chromium.org/2111103002/diff/40001/chrome/browser/password...
File chrome/browser/password_manager/password_store_mac.cc (right):

https://codereview.chromium.org/2111103002/diff/40001/chrome/browser/password...
chrome/browser/password_manager/password_store_mac.cc:282: // time_string_bytes
was parsable. If the return value is false, the value of
On 2016/07/05 10:59:53, vabr (Chromium) wrote:
> The last sentence of this comment ("If the return ... is unchanged.") is no
> longer true, please remove it. I checked the (only) callsite and it does not
> seem to rely on this promise, so just dropping the comment is OK.

Done.

[Lei Zhang, 2016-07-06 17:44:30]

https://codereview.chromium.org/2111103002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/device_status_collector.cc (right):

https://codereview.chromium.org/2111103002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_status_collector.cc:108: if
(!Time::FromUTCExploded(exploded, &out_time)) {
Can values that come from another Time instance ever be bad?

https://codereview.chromium.org/2111103002/diff/60001/chrome/browser/password...
File chrome/browser/password_manager/password_store_mac.cc (right):

https://codereview.chromium.org/2111103002/diff/60001/chrome/browser/password...
chrome/browser/password_manager/password_store_mac.cc:300: if (assignments == 6)
{
This can just be:
return assignments == 6 && base::Time::FromUTCExploded(exploded_time, time);

https://codereview.chromium.org/2111103002/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/media/webrtc_logs_ui.cc (right):

https://codereview.chromium.org/2111103002/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/media/webrtc_logs_ui.cc:186: if
(base::Time::FromUTCExploded(lower_limit, &out_time)) {
Isn't |lower_limit| a known to be good value that should never fail? Can we
DCHECK() that the conversion is successful?

|lower_limit| should also be const.

https://codereview.chromium.org/2111103002/diff/60001/chrome/installer/gcapi/...
File chrome/installer/gcapi/gcapi_omaha_experiment.cc (right):

https://codereview.chromium.org/2111103002/diff/60001/chrome/installer/gcapi/...
chrome/installer/gcapi/gcapi_omaha_experiment.cc:21: base::Time::Exploded
february_third_2003_exploded =
const, if you don't mind.

https://codereview.chromium.org/2111103002/diff/60001/chrome/utility/media_ga...
File chrome/utility/media_galleries/picasa_album_table_reader.cc (right):

https://codereview.chromium.org/2111103002/diff/60001/chrome/utility/media_ga...
chrome/utility/media_galleries/picasa_album_table_reader.cc:28: if
(!base::Time::FromLocalExploded(kPmpVariantTimeEpoch, &out_time)) {
Isn't |kPmpVariantTimeEpoch| also a known to be good date that should no fail?

[maksims (do not use this acc), 2016-07-07 05:48:14]

https://codereview.chromium.org/2111103002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/device_status_collector.cc (right):

https://codereview.chromium.org/2111103002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_status_collector.cc:108: if
(!Time::FromUTCExploded(exploded, &out_time)) {
On 2016/07/06 17:44:29, Lei Zhang wrote:
> Can values that come from another Time instance ever be bad?

No, they shouldn't

https://codereview.chromium.org/2111103002/diff/60001/chrome/browser/password...
File chrome/browser/password_manager/password_store_mac.cc (right):

https://codereview.chromium.org/2111103002/diff/60001/chrome/browser/password...
chrome/browser/password_manager/password_store_mac.cc:300: if (assignments == 6)
{
On 2016/07/06 17:44:30, Lei Zhang wrote:
> This can just be:
> return assignments == 6 && base::Time::FromUTCExploded(exploded_time, time);

Done.

https://codereview.chromium.org/2111103002/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/media/webrtc_logs_ui.cc (right):

https://codereview.chromium.org/2111103002/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/media/webrtc_logs_ui.cc:186: if
(base::Time::FromUTCExploded(lower_limit, &out_time)) {
On 2016/07/06 17:44:30, Lei Zhang wrote:
> Isn't |lower_limit| a known to be good value that should never fail? Can we
> DCHECK() that the conversion is successful?
> 
> |lower_limit| should also be const.

Done.

https://codereview.chromium.org/2111103002/diff/60001/chrome/installer/gcapi/...
File chrome/installer/gcapi/gcapi_omaha_experiment.cc (right):

https://codereview.chromium.org/2111103002/diff/60001/chrome/installer/gcapi/...
chrome/installer/gcapi/gcapi_omaha_experiment.cc:21: base::Time::Exploded
february_third_2003_exploded =
On 2016/07/06 17:44:30, Lei Zhang wrote:
> const, if you don't mind.

Done.

https://codereview.chromium.org/2111103002/diff/60001/chrome/utility/media_ga...
File chrome/utility/media_galleries/picasa_album_table_reader.cc (right):

https://codereview.chromium.org/2111103002/diff/60001/chrome/utility/media_ga...
chrome/utility/media_galleries/picasa_album_table_reader.cc:28: if
(!base::Time::FromLocalExploded(kPmpVariantTimeEpoch, &out_time)) {
On 2016/07/06 17:44:30, Lei Zhang wrote:
> Isn't |kPmpVariantTimeEpoch| also a known to be good date that should no fail?

Done.

[Lei Zhang, 2016-07-07 05:50:55]

lgtm

https://codereview.chromium.org/2111103002/diff/60001/chrome/installer/gcapi/...
File chrome/installer/gcapi/gcapi_omaha_experiment.cc (right):

https://codereview.chromium.org/2111103002/diff/60001/chrome/installer/gcapi/...
chrome/installer/gcapi/gcapi_omaha_experiment.cc:21: base::Time::Exploded
february_third_2003_exploded =
On 2016/07/07 05:48:14, maksims wrote:
> On 2016/07/06 17:44:30, Lei Zhang wrote:
> > const, if you don't mind.
> 
> Done.

Thanks!

[maksims (do not use this acc), 2016-07-07 08:51:40]

The CQ bit was checked by maksim.sisov@intel.com

[maksims (do not use this acc), 2016-07-07 08:51:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
benwells@chromium.org, jyquinn@chromium.org, tsergeant@chromium.org,
vabr@chromium.org
Link to the patchset: https://codereview.chromium.org/2111103002/#ps80001
(title: "Lei Zhang's comments")

[commit-bot: I haz the power, 2016-07-07 08:51:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-07 09:17:40]

Description was changed from

==========
Make callers of FromUTC(Local)Exploded in chrome/ use new time API. 

Use new time conversion API in accordance with 
https://codereview.chromium.org/1988663002/

BUG=601900
==========

to

==========
Make callers of FromUTC(Local)Exploded in chrome/ use new time API. 

Use new time conversion API in accordance with 
https://codereview.chromium.org/1988663002/

BUG=601900
==========

[commit-bot: I haz the power, 2016-07-07 09:17:42]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-07-07 09:19:18]

Description was changed from

==========
Make callers of FromUTC(Local)Exploded in chrome/ use new time API. 

Use new time conversion API in accordance with 
https://codereview.chromium.org/1988663002/

BUG=601900
==========

to

==========
Make callers of FromUTC(Local)Exploded in chrome/ use new time API.

Use new time conversion API in accordance with
https://codereview.chromium.org/1988663002/

BUG=601900

Committed: https://crrev.com/b4dbc73df4c9c1c9bc3439d4c0f79c79b64f18f4
Cr-Commit-Position: refs/heads/master@{#404106}
==========

[commit-bot: I haz the power, 2016-07-07 09:19:20]

Patchset 3 (id:??) landed as
https://crrev.com/b4dbc73df4c9c1c9bc3439d4c0f79c79b64f18f4
Cr-Commit-Position: refs/heads/master@{#404106}