[Chromecast] Allow media metrics events with no app running

[Chromecast] Allow media metrics events with no session ID set

If a session ID is not set, we hit an assert sometimes in browser tests:

FATAL:hash_util.cc(78)] Check failed: hex.size() == 32u (0vs. 32)
#2 0x00000450354e cast::HashGUID64()
#3 0x000004503895 cast::HashSessionId64()
#4 0x0000044960d1 chromecast::CastEventBuilderImpl::SetSessionId()
#5 0x000004496530 chromecast::MetricsRecorderImpl::RecordSimpleAction()

Set a default session ID to prevent this from being reached.

BUG=

Committed: https://crrev.com/ba0fc7ff60bec8b1461c997dcb206f5c6c3249ce
Cr-Commit-Position: refs/heads/master@{#362421}

[halliwell, 2015-11-25 01:13:59]

Description was changed from

==========
[Chromecast] Allow media metrics events with no app running

In trying to get browser tests off the ground against internal code,
I hit this crash:

FATAL:hash_util.cc(78)] Check failed: hex.size() == 32u (0vs. 32)
...

Longer-term, this might be a bit of a losing battle, and perhaps the
test code should run everything as an app, but given this is the only
blocker right now, seems simpler to fix this.

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

to

==========
[Chromecast] Allow media metrics events with no app running

In trying to get browser tests off the ground against internal code,
I hit this crash:

FATAL:hash_util.cc(78)] Check failed: hex.size() == 32u (0vs. 32)
#2 0x00000450354e cast::HashGUID64()                                            
                                                                              
#3 0x000004503895 cast::HashSessionId64()                                       
                                                                              
#4 0x0000044960d1 chromecast::CastEventBuilderImpl::SetSessionId()              
                                                                              
#5 0x000004496530 chromecast::MetricsRecorderImpl::RecordSimpleAction()

Longer-term, this might be a bit of a losing battle, and perhaps the
test code should run everything as an app, but given this is the only
blocker right now, seems simpler to fix this.

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

[halliwell, 2015-11-25 01:14:16]

halliwell@chromium.org changed reviewers:
+ mbjorge@chromium.org, slan@chromium.org

[slan, 2015-11-25 01:22:25]

https://codereview.chromium.org/1477483003/diff/1/chromecast/base/metrics/cas...
File chromecast/base/metrics/cast_metrics_helper.cc (right):

https://codereview.chromium.org/1477483003/diff/1/chromecast/base/metrics/cas...
chromecast/base/metrics/cast_metrics_helper.cc:131: if (app_id_.empty())
Did you consider setting a testing value for app_id_? That way we can follow
this codepath all the way down in our tests.

[halliwell, 2015-11-25 02:57:10]

On 2015/11/25 01:22:25, slan wrote:
>
https://codereview.chromium.org/1477483003/diff/1/chromecast/base/metrics/cas...
> File chromecast/base/metrics/cast_metrics_helper.cc (right):
> 
>
https://codereview.chromium.org/1477483003/diff/1/chromecast/base/metrics/cas...
> chromecast/base/metrics/cast_metrics_helper.cc:131: if (app_id_.empty())
> Did you consider setting a testing value for app_id_? That way we can follow
> this codepath all the way down in our tests.

I don't know much about this code tbh, mbjorge might be better placed to answer?
 I went with this approach because other functions in the class are checking
app_id_.  Also, it's the session_id_ that causes the assertion.

[mbjorge, 2015-11-25 21:42:41]

Alternative option, could we give session_id_ a default value upon construction?
e.g. '0x0000-0000-0000-0000-0000-0000-0000-00'?

[halliwell, 2015-11-25 23:58:53]

On 2015/11/25 21:42:41, mbjorge wrote:
> Alternative option, could we give session_id_ a default value upon
construction?
> e.g. '0x0000-0000-0000-0000-0000-0000-0000-00'?

Ok, set a default session_id and reverted my app_id checks.  This fixes the
problem in the test equally well.

[halliwell, 2015-11-26 00:00:11]

Description was changed from

==========
[Chromecast] Allow media metrics events with no app running

In trying to get browser tests off the ground against internal code,
I hit this crash:

FATAL:hash_util.cc(78)] Check failed: hex.size() == 32u (0vs. 32)
#2 0x00000450354e cast::HashGUID64()                                            
                                                                              
#3 0x000004503895 cast::HashSessionId64()                                       
                                                                              
#4 0x0000044960d1 chromecast::CastEventBuilderImpl::SetSessionId()              
                                                                              
#5 0x000004496530 chromecast::MetricsRecorderImpl::RecordSimpleAction()

Longer-term, this might be a bit of a losing battle, and perhaps the
test code should run everything as an app, but given this is the only
blocker right now, seems simpler to fix this.

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

to

==========
[Chromecast] Allow media metrics events with no session ID set

If a session ID is not set, we hit an assert sometimes in browser tests:

FATAL:hash_util.cc(78)] Check failed: hex.size() == 32u (0vs. 32)
#2 0x00000450354e cast::HashGUID64()                                            
                                                                              
#3 0x000004503895 cast::HashSessionId64()                                       
                                                                              
#4 0x0000044960d1 chromecast::CastEventBuilderImpl::SetSessionId()              
                                                                              
#5 0x000004496530 chromecast::MetricsRecorderImpl::RecordSimpleAction()

Set a default session ID to prevent this from being reached.

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

[mbjorge, 2015-11-26 00:04:56]

lgtm

[halliwell, 2015-11-26 00:08:12]

The CQ bit was checked by halliwell@chromium.org

[commit-bot: I haz the power, 2015-11-26 00:09:27]

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

[commit-bot: I haz the power, 2015-11-26 02:13:52]

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

[commit-bot: I haz the power, 2015-11-26 02:13:53]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_TIMED_OUT, no build
URL)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)

[slan, 2015-11-30 18:51:44]

lgtm

[halliwell, 2015-12-01 15:31:26]

The CQ bit was checked by halliwell@chromium.org

[commit-bot: I haz the power, 2015-12-01 15:33:21]

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

[commit-bot: I haz the power, 2015-12-01 16:26:40]

Description was changed from

==========
[Chromecast] Allow media metrics events with no session ID set

If a session ID is not set, we hit an assert sometimes in browser tests:

FATAL:hash_util.cc(78)] Check failed: hex.size() == 32u (0vs. 32)
#2 0x00000450354e cast::HashGUID64()                                            
                                                                              
#3 0x000004503895 cast::HashSessionId64()                                       
                                                                              
#4 0x0000044960d1 chromecast::CastEventBuilderImpl::SetSessionId()              
                                                                              
#5 0x000004496530 chromecast::MetricsRecorderImpl::RecordSimpleAction()

Set a default session ID to prevent this from being reached.

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

to

==========
[Chromecast] Allow media metrics events with no session ID set

If a session ID is not set, we hit an assert sometimes in browser tests:

FATAL:hash_util.cc(78)] Check failed: hex.size() == 32u (0vs. 32)
#2 0x00000450354e cast::HashGUID64()                                            
                                                                              
#3 0x000004503895 cast::HashSessionId64()                                       
                                                                              
#4 0x0000044960d1 chromecast::CastEventBuilderImpl::SetSessionId()              
                                                                              
#5 0x000004496530 chromecast::MetricsRecorderImpl::RecordSimpleAction()

Set a default session ID to prevent this from being reached.

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

[commit-bot: I haz the power, 2015-12-01 16:26:41]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-12-01 16:28:18]

Description was changed from

==========
[Chromecast] Allow media metrics events with no session ID set

If a session ID is not set, we hit an assert sometimes in browser tests:

FATAL:hash_util.cc(78)] Check failed: hex.size() == 32u (0vs. 32)
#2 0x00000450354e cast::HashGUID64()                                            
                                                                              
#3 0x000004503895 cast::HashSessionId64()                                       
                                                                              
#4 0x0000044960d1 chromecast::CastEventBuilderImpl::SetSessionId()              
                                                                              
#5 0x000004496530 chromecast::MetricsRecorderImpl::RecordSimpleAction()

Set a default session ID to prevent this from being reached.

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

to

==========
[Chromecast] Allow media metrics events with no session ID set

If a session ID is not set, we hit an assert sometimes in browser tests:

FATAL:hash_util.cc(78)] Check failed: hex.size() == 32u (0vs. 32)
#2 0x00000450354e cast::HashGUID64()
#3 0x000004503895 cast::HashSessionId64()
#4 0x0000044960d1 chromecast::CastEventBuilderImpl::SetSessionId()
#5 0x000004496530 chromecast::MetricsRecorderImpl::RecordSimpleAction()

Set a default session ID to prevent this from being reached.

BUG=

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

[commit-bot: I haz the power, 2015-12-01 16:28:19]

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