Sotaro Ikeda [:sotaro] Assignee
	Description • 6 years ago

Created by Bug 1481950 Comment 1.

	Sotaro Ikeda [:sotaro] Assignee
	Comment 2 • 6 years ago

With attachment 8998733 [details] [diff] [review], WrBridge()->SendSetFocusTarget() is still called. It could be handled as a different problem.

	Sotaro Ikeda [:sotaro] Assignee
	Comment 5 • 6 years ago

By Bug 1481950 comment 3, attachment 8998734 [details] [diff] [review] might affect to MozAfterPaint.

	Sotaro Ikeda [:sotaro] Assignee
	Comment 6 • 6 years ago

(In reply to Sotaro Ikeda [:sotaro] from comment #5)
> By Bug 1481950 comment 3, attachment 8998734 [details] [diff] [review] might
> affect to MozAfterPaint.

Copy of Bug 1481950 comment 3.

---------------------------------------------

Thanks Sotaro!

It might also make sense to only register a value for CONTENT_FRAME_TIME if we actually drew something. Both backends seem to have fallbacks path where we call DidComposite/NotifyPipelineRendered even when no work was done.

I think we need some code to run to ensure MozAfterPaint is delivered (since JS can be waiting for it), but we should be able to skip recording the telemetry value.

	Sotaro Ikeda [:sotaro] Assignee
	Comment 7 • 6 years ago

Changed summary by comment 6.

	Sotaro Ikeda [:sotaro] Assignee
	Comment 10 • 6 years ago

Update a comment.

	Sotaro Ikeda [:sotaro] Assignee
	Comment 11 • 6 years ago

Comment on attachment 8999506 [details] [diff] [review]
patch part 2 - Only register a value for CONTENT_FRAME_TIME if we actually drew something

:mattwoodrow, can you feedback to the patch?

	Matt Woodrow (:mattwoodrow)
	Comment 12 • 6 years ago

Comment on attachment 8999506 [details] [diff] [review]
patch part 2 - Only register a value for CONTENT_FRAME_TIME if we actually drew something

Review of attachment 8999506 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!

Have you checked in about:telemetry to see if the low values go away?

	Sotaro Ikeda [:sotaro] Assignee
	Comment 13 • 6 years ago

(In reply to Matt Woodrow (:mattwoodrow) from comment #12)
> 
> Have you checked in about:telemetry to see if the low values go away?

Yes, I checked the CONTENT_FRAME_TIME in about:telemetry. The very low values went away.


Before applying the patches, CONTENT_FRAME_TIME was like the following.

****************************************
CONTENT_FRAME_TIME
159 samples, average = 107.3, sum = 17068

  2 |  0  0%
  3 |#  3  2%
  4 |##  6  4%
  5 |#  4  3%
  9 |  1  1%
 11 |  1  1%
 13 |#  4  3%
 15 |#  3  2%
 21 |  1  1%
 40 |  1  1%
 55 |#  2  1%
 64 |#  3  2%
 75 |#  3  2%
 88 |#  3  2%
103 |#########################  73  46%
120 |##########  29  18%
140 |###  9  6%
164 |##  5  3%
192 |#  4  3%
224 |#  3  2%
357 |  1  1%
417 |  0  0%

****************************************


After applying the patches, CONTENT_FRAME_TIME was like the following. The very low values went away.

****************************************
CONTENT_FRAME_TIME
187 samples, average = 122.3, sum = 22868

  9 |  0  0%
 11 |  1  1%
 34 |  1  1%
 64 |#  5  3%
 75 |#  4  2%
 88 |###  10  5%
103 |#########################  100  53%
120 |###########  43  23%
140 |##  8  4%
164 |##  6  3%
192 |#  4  2%
224 |#  3  2%
262 |  1  1%
417 |  1  1%
487 |  0  0%


****************************************

	Sotaro Ikeda [:sotaro] Assignee
	Comment 14 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4945fee0ac919795e6c1b8c88fce54b92495e319

	Pulsebot
	Comment 15 • 6 years ago

Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1f4543fd3fb
part 1 - A bit clean up of FlushTransactionIdsForEpoch() r=mattwoodrow
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc38e7f7ca73
part 2 - Only register a value for CONTENT_FRAME_TIME if we actually drew something r=mattwoodrow

	Narcis Beleuzu [:NarcisB]
	Comment 16 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/c1f4543fd3fb
https://hg.mozilla.org/mozilla-central/rev/bc38e7f7ca73