Add Foreground-to-FirstMeaningfulPaint histogram

Add Foreground-to-FirstMeaningfulPaint histogram

This metric measures the time between background tab being foregrounded
and the first meaningful paint. This would help to understand the impact
of loading request throttling for background tabs.

BUG=729951
Review-Url: https://codereview.chromium.org/2930583002
Cr-Commit-Position: refs/heads/master@{#478166}
Committed: https://chromium.googlesource.com/chromium/src/+/aa319f6ec07a46c2e9d0b56bb22baad5e407c939

[Kunihiko Sakamoto, 2017-06-07 02:37:12]

The CQ bit was checked by ksakamoto@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-07 02:37:28]

Dry run: 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, 2017-06-07 04:19:45]

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

[commit-bot: I haz the power, 2017-06-07 04:19:46]

Dry run: This issue passed the CQ dry run.

[Kunihiko Sakamoto, 2017-06-07 07:29:45]

Patchset #1 (id:1) has been deleted

[Kunihiko Sakamoto, 2017-06-07 07:39:27]

Description was changed from

==========
Add ForegroundToFirstMeaningfulPaint histogram

BUG=729951
==========

to

==========
Add Foreground-to-FirstMeaningfulPaint histogram

This metric measures the time between background tab being foregrounded
and the first meaningful paint. This would help to understand the impact
of loading request throttling for background tabs.

BUG=729951
==========

[Kunihiko Sakamoto, 2017-06-07 07:45:34]

ksakamoto@chromium.org changed reviewers:
+ toyoshim@chromium.org

[Takashi Toyoshima, 2017-06-07 08:39:38]

lg with one suggestion

https://codereview.chromium.org/2930583002/diff/20001/chrome/browser/page_loa...
File
chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
(right):

https://codereview.chromium.org/2930583002/diff/20001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc:502:
// Record the time to first meaningful paint for pages which were:
I can not find any problem, but conditions look complicated for me.
Probably, we have mixed two condition set together. One is for FMP check, and
the other is for ForegroundTo condition. If we could factor out ForegroundTo
check to be shared with ForegroundToFP, that would increase readability a
little?

[Kunihiko Sakamoto, 2017-06-07 09:20:51]

ksakamoto@chromium.org changed reviewers:
+ bmcquade@chromium.org, isherman@chromium.org

[Kunihiko Sakamoto, 2017-06-07 09:20:52]

+bmcquade@ for page_load_metrics
+isherman@ for histograms.xml

PTAL

https://codereview.chromium.org/2930583002/diff/20001/chrome/browser/page_loa...
File
chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
(right):

https://codereview.chromium.org/2930583002/diff/20001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc:502:
// Record the time to first meaningful paint for pages which were:
On 2017/06/07 08:39:38, Takashi Toyoshima wrote:
> I can not find any problem, but conditions look complicated for me.
> Probably, we have mixed two condition set together. One is for FMP check, and
> the other is for ForegroundTo condition. If we could factor out ForegroundTo
> check to be shared with ForegroundToFP, that would increase readability a
> little?

Done.

[kinuko, 2017-06-07 09:45:35]

Non-real-reviewer lgtm, cool!

[tdresser, 2017-06-07 12:19:05]

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

[tdresser, 2017-06-07 12:19:05]

Could we add an FCP variant as well?

[Bryan McQuade, 2017-06-07 13:55:18]

On 2017/06/07 at 12:19:05, tdresser wrote:
> Could we add an FCP variant as well?

Agree with the suggestion to add an FCP variant while making this change. Change
LGTM overall, thank you!

[Ilya Sherman, 2017-06-07 21:09:38]

Metrics LGTM

[Kunihiko Sakamoto, 2017-06-08 03:33:42]

The CQ bit was checked by ksakamoto@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-08 03:33:53]

Dry run: CQ is trying da patch.

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

[Kunihiko Sakamoto, 2017-06-08 03:36:41]

On 2017/06/07 12:19:05, tdresser wrote:
> Could we add an FCP variant as well?

OK, added FCP variant as well.

[Ilya Sherman, 2017-06-08 03:53:42]

(Metrics still LGTM)

[Kunihiko Sakamoto, 2017-06-08 04:46:09]

Kinuko-san, PTAL for PS 2 to 3 change, which is simple addition of FG-to-FCP
histogram.

[commit-bot: I haz the power, 2017-06-08 04:56:43]

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

[commit-bot: I haz the power, 2017-06-08 04:56:44]

Dry run: This issue passed the CQ dry run.

[kinuko, 2017-06-09 02:25:26]

lgtm!

[kinuko, 2017-06-09 02:25:32]

lgtm

[Kunihiko Sakamoto, 2017-06-09 02:28:05]

The CQ bit was checked by ksakamoto@chromium.org

[Kunihiko Sakamoto, 2017-06-09 02:28:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmcquade@chromium.org
Link to the patchset: https://codereview.chromium.org/2930583002/#ps60001
(title: "add FCP variant")

[commit-bot: I haz the power, 2017-06-09 02:28:21]

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, 2017-06-09 02:32:41]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1496975285647830,
"parent_rev": "b9d91304717241346238f1d50061f829e2bc2b30", "commit_rev":
"aa319f6ec07a46c2e9d0b56bb22baad5e407c939"}

[commit-bot: I haz the power, 2017-06-09 02:32:52]

Description was changed from

==========
Add Foreground-to-FirstMeaningfulPaint histogram

This metric measures the time between background tab being foregrounded
and the first meaningful paint. This would help to understand the impact
of loading request throttling for background tabs.

BUG=729951
==========

to

==========
Add Foreground-to-FirstMeaningfulPaint histogram

This metric measures the time between background tab being foregrounded
and the first meaningful paint. This would help to understand the impact
of loading request throttling for background tabs.

BUG=729951

Review-Url: https://codereview.chromium.org/2930583002
Cr-Commit-Position: refs/heads/master@{#478166}
Committed:
https://chromium.googlesource.com/chromium/src/+/aa319f6ec07a46c2e9d0b56bb22b...
==========

[commit-bot: I haz the power, 2017-06-09 02:32:53]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/aa319f6ec07a46c2e9d0b56bb22b...