Change dont_embed_build_metadata default to 1 except for Official build and fix some unittests.

Change dont_embed_build_metadata default to 1 except for Official build and fix some unittests.

FieldTrialTest.DisableProbability was using Time::NowFromSystemTime - 1 year to generate a date before the build time. Instead use GetBuildTime - 1 year.

TransportSecurityState::IsBuildTimely is using GetBuildTime to determine if the build is older than 10 weeks. Disable this and return a default value if DONT_EMBED_BUILD_METADATA is defined (i.e. The build time is invalid) and we're not doing an official build.

CL based on https://codereview.chromium.org/685123005/ with some additional fixes for the unittests.

BUG=314403
Committed: https://crrev.com/153e1b83e8af5e6f9dbd3b79e5e0643ea8bcf72c
Cr-Commit-Position: refs/heads/master@{#302316}

[Sébastien Marchand, 2014-10-30 16:12:45]

sebmarchand@chromium.org changed reviewers:
+ agl@chromium.org, asvitkine@chromium.org, maruel@chromium.org,
thakis@chromium.org

[Sébastien Marchand, 2014-10-30 16:14:00]

PTAL.

[M-A Ruel, 2014-10-30 16:19:53]

lgtm thanks, that will save a lot of infrastructure usage with redundant test
execution.

[Alexei Svitkine (slow), 2014-10-30 16:26:02]

LGTM

[Nico, 2014-10-30 16:56:15]

lgtm, nice :-)

Please make sure that you don't get the hardcoded build date in official builds.

https://codereview.chromium.org/695523002/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/695523002/diff/1/build/common.gypi#newcode1509
build/common.gypi:1509: # generate anything deterministic with WPO enabled
AFAIK.
Could we have grit expand the current time and load the build time from a
resource? Then executable builds would be repeatable at least, only the resource
file would change.

[M-A Ruel, 2014-10-30 17:03:14]

https://codereview.chromium.org/695523002/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/695523002/diff/1/build/common.gypi#newcode1509
build/common.gypi:1509: # generate anything deterministic with WPO enabled
AFAIK.
On 2014/10/30 16:56:15, Nico wrote:
> Could we have grit expand the current time and load the build time from a
> resource? Then executable builds would be repeatable at least, only the
resource
> file would change.

That would still change the .isolated content, so no for TS and CI. For
official, I agree this would be a gain security wise (same executable bits) and
would likely the only doable route but it's still quite far away.

If this is done eventually done for official build (which is out of scope for
now), this would likely have the side effect of reducing the incremental upgrade
size but that would have to be measured.

[Nico, 2014-10-30 17:04:21]

On Thu, Oct 30, 2014 at 10:03 AM, <maruel@chromium.org> wrote:

>
> https://codereview.chromium.org/695523002/diff/1/build/common.gypi
> File build/common.gypi (right):
>
> https://codereview.chromium.org/695523002/diff/1/build/
> common.gypi#newcode1509
> build/common.gypi:1509: # generate anything deterministic with WPO
> enabled AFAIK.
> On 2014/10/30 16:56:15, Nico wrote:
>
>> Could we have grit expand the current time and load the build time
>>
> from a
>
>> resource? Then executable builds would be repeatable at least, only
>>
> the resource
>
>> file would change.
>>
>
> That would still change the .isolated content, so no for TS and CI. For
> official, I agree this would be a gain security wise (same executable
> bits) and would likely the only doable route but it's still quite far
> away.
>

Yes, I meant for official only.

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[agl, 2014-10-30 17:11:29]

LGTM for net/

[Sébastien Marchand, 2014-10-30 17:16:11]

The CQ bit was checked by sebmarchand@chromium.org

[commit-bot: I haz the power, 2014-10-30 17:17:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/695523002/20001

[commit-bot: I haz the power, 2014-10-30 21:03:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-30 21:03:05]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[M-A Ruel, 2014-10-30 21:09:07]

https://codereview.chromium.org/695523002/diff/40001/chrome/browser/ssl/ssl_e...
File chrome/browser/ssl/ssl_error_classification.cc (right):

https://codereview.chromium.org/695523002/diff/40001/chrome/browser/ssl/ssl_e...
chrome/browser/ssl/ssl_error_classification.cc:356: #if defined(OFFICIAL_BUILD)
Why not use for consistency?
#if defined(DONT_EMBED_BUILD_METADATA) && !defined(OFFICIAL_BUILD)

[Sébastien Marchand, 2014-10-30 21:09:41]

I had to fix a flakiness in ssl_error_classification.cc, agl@, can you take
another look please (for browser/ssl) ?

[M-A Ruel, 2014-10-30 21:13:06]

chrome/browser/ssl/ssl_error_classification.cc lgtm

[Sébastien Marchand, 2014-10-30 21:14:06]

Addressed maruel's comment.

https://codereview.chromium.org/695523002/diff/40001/chrome/browser/ssl/ssl_e...
File chrome/browser/ssl/ssl_error_classification.cc (right):

https://codereview.chromium.org/695523002/diff/40001/chrome/browser/ssl/ssl_e...
chrome/browser/ssl/ssl_error_classification.cc:356: #if defined(OFFICIAL_BUILD)
On 2014/10/30 21:09:07, M-A Ruel wrote:
> Why not use for consistency?
> #if defined(DONT_EMBED_BUILD_METADATA) && !defined(OFFICIAL_BUILD)

Done.

[Sébastien Marchand, 2014-10-30 21:50:44]

Patchset #5 (id:80001) has been deleted

[Sébastien Marchand, 2014-10-30 21:51:51]

I've fixed the logic in ssl_error_classification, I was doing the opposite of
what I wanted to do :)

[M-A Ruel, 2014-10-30 22:53:49]

On 2014/10/30 21:51:51, Sébastien Marchand wrote:
> I've fixed the logic in ssl_error_classification, I was doing the opposite of
> what I wanted to do :)

lgtm with logic fixed, sorry for missing it.

[Sébastien Marchand, 2014-10-31 00:11:28]

Thanks, agl@, if this lgty feel free to flip the CQ bit.

[agl2, 2014-10-31 18:12:00]

agl@google.com changed reviewers:
+ agl@google.com

[agl2, 2014-10-31 18:12:01]

LGTM

[Sébastien Marchand, 2014-10-31 18:12:44]

The CQ bit was checked by sebmarchand@chromium.org

[commit-bot: I haz the power, 2014-10-31 18:14:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/695523002/100001

[commit-bot: I haz the power, 2014-10-31 20:26:17]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2014-10-31 20:26:42]

Patchset 5 (id:??) landed as
https://crrev.com/153e1b83e8af5e6f9dbd3b79e5e0643ea8bcf72c
Cr-Commit-Position: refs/heads/master@{#302316}