Don't restart the hang renderer timeout on messages ack'd from the compositor thread.

Don't restart the hang renderer timeout on messages ack'd from the compositor thread.

Only reset the hang monitor timeout based on an ack actually coming
from the main thread. This way we can put the dialog up even if the
compositor is responsive but the main thread isn't.

BUG=654835
Committed: https://crrev.com/120e997fc114e87d1b98a32ac9a81bce954e05d2
Cr-Commit-Position: refs/heads/master@{#430355}

[dtapuska, 2016-11-04 18:05:39]

dtapuska@chromium.org changed reviewers:
+ creis@chromium.org, tdresser@chromium.org

[dtapuska, 2016-11-04 18:05:40]

tdresser@chromium.org: Please review changes in 
*/input/*

creis@chromium.org: Please review changes in 
content/*

[dtapuska, 2016-11-04 18:06:26]

dtapuska@chromium.org changed reviewers:
+ dcheng@chromium.org

[dtapuska, 2016-11-04 18:06:27]

dcheng@ please review input_messages.h

[dcheng, 2016-11-04 18:24:49]

ipc lgtm with nits addressed

https://codereview.chromium.org/2482453002/diff/1/content/common/input/input_...
File content/common/input/input_event_ack_location.h (right):

https://codereview.chromium.org/2482453002/diff/1/content/common/input/input_...
content/common/input/input_event_ack_location.h:13:
INPUT_EVENT_ACK_LOCATION_UNKNOWN,
Nit: since this is an enum class, just omit the INPUT_EVENT_ACK_LOCATION_ prefix

https://codereview.chromium.org/2482453002/diff/1/content/renderer/mus/compos...
File content/renderer/mus/compositor_mus_connection_unittest.cc (right):

https://codereview.chromium.org/2482453002/diff/1/content/renderer/mus/compos...
content/renderer/mus/compositor_mus_connection_unittest.cc:210:
content::InputEventAckLocation::
Nit: though it's not really changed by this CL, consider moving this unnamed
namespace into namespace content below (that way, we won't have to specify the
content namespace so much in this code)

[tdresser, 2016-11-04 18:35:54]

I didn't see any tests checking that this is set correctly.

LGTM with a test.

https://codereview.chromium.org/2482453002/diff/1/content/common/input/input_...
File content/common/input/input_event_ack_location.h (right):

https://codereview.chromium.org/2482453002/diff/1/content/common/input/input_...
content/common/input/input_event_ack_location.h:12: enum class
InputEventAckLocation {
I'd prefer InputEventAckSource, as it makes it clear you're referring to where
the ack was generated, rather than where it currently is.

This comment is a bit confusing.

Where the ack was "generated to" the browser? What does that mean?

[dtapuska, 2016-11-04 20:13:47]

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

[dtapuska, 2016-11-04 20:14:17]

https://codereview.chromium.org/2482453002/diff/1/content/common/input/input_...
File content/common/input/input_event_ack_location.h (right):

https://codereview.chromium.org/2482453002/diff/1/content/common/input/input_...
content/common/input/input_event_ack_location.h:12: enum class
InputEventAckLocation {
On 2016/11/04 18:35:54, tdresser wrote:
> I'd prefer InputEventAckSource, as it makes it clear you're referring to where
> the ack was generated, rather than where it currently is.
> 
> This comment is a bit confusing.
> 
> Where the ack was "generated to" the browser? What does that mean?

Done.

https://codereview.chromium.org/2482453002/diff/1/content/common/input/input_...
content/common/input/input_event_ack_location.h:13:
INPUT_EVENT_ACK_LOCATION_UNKNOWN,
On 2016/11/04 18:24:49, dcheng wrote:
> Nit: since this is an enum class, just omit the INPUT_EVENT_ACK_LOCATION_
prefix

Done.

https://codereview.chromium.org/2482453002/diff/1/content/renderer/mus/compos...
File content/renderer/mus/compositor_mus_connection_unittest.cc (right):

https://codereview.chromium.org/2482453002/diff/1/content/renderer/mus/compos...
content/renderer/mus/compositor_mus_connection_unittest.cc:210:
content::InputEventAckLocation::
On 2016/11/04 18:24:49, dcheng wrote:
> Nit: though it's not really changed by this CL, consider moving this unnamed
> namespace into namespace content below (that way, we won't have to specify the
> content namespace so much in this code)

Done.

[commit-bot: I haz the power, 2016-11-04 20:14:38]

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, 2016-11-04 20:46:25]

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

[commit-bot: I haz the power, 2016-11-04 20:46:26]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Charlie Reis, 2016-11-04 21:14:17]

Thanks.  RS LGTM for content/, but please note that tdresser@ requested a test.

https://codereview.chromium.org/2482453002/diff/20001/content/common/BUILD.gn
File content/common/BUILD.gn (right):

https://codereview.chromium.org/2482453002/diff/20001/content/common/BUILD.gn...
content/common/BUILD.gn:163: "input/input_event_ack_state.h",
Was this one just missing before?  I see the file is already in the repository.

[dtapuska, 2016-11-07 16:41:27]

tdresser@ ptal I've added the unit test you requested.

https://codereview.chromium.org/2482453002/diff/20001/content/common/BUILD.gn
File content/common/BUILD.gn (right):

https://codereview.chromium.org/2482453002/diff/20001/content/common/BUILD.gn...
content/common/BUILD.gn:163: "input/input_event_ack_state.h",
On 2016/11/04 21:14:17, Charlie Reis wrote:
> Was this one just missing before?  I see the file is already in the
repository.

Yup it was missing

[tdresser, 2016-11-07 16:44:24]

LGTM, thanks.

[dtapuska, 2016-11-07 16:47:12]

The CQ bit was checked by dtapuska@chromium.org

[dtapuska, 2016-11-07 16:47:13]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org,
creis@chromium.org
Link to the patchset: https://codereview.chromium.org/2482453002/#ps40001
(title: "Add unit test")

[commit-bot: I haz the power, 2016-11-07 16:47:34]

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-11-07 17:34:01]

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

[commit-bot: I haz the power, 2016-11-07 17:34:03]

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

[dtapuska, 2016-11-07 17:57:50]

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

[commit-bot: I haz the power, 2016-11-07 17:58:20]

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, 2016-11-07 19:26:08]

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

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

Dry run: This issue passed the CQ dry run.

[dtapuska, 2016-11-07 20:06:09]

The CQ bit was checked by dtapuska@chromium.org

[dtapuska, 2016-11-07 20:06:10]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org,
tdresser@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2482453002/#ps60001
(title: "Fix mac build")

[commit-bot: I haz the power, 2016-11-07 20:06:38]

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-11-07 20:14:17]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-07 20:22:14]

Description was changed from

==========
Don't restart the hang renderer timeout on messages ack'd from the compositor
thread.

Only reset the hang monitor timeout based on an ack actually coming
from the main thread. This way we can put the dialog up even if the
compositor is responsive but the main thread isn't.

BUG=654835
==========

to

==========
Don't restart the hang renderer timeout on messages ack'd from the compositor
thread.

Only reset the hang monitor timeout based on an ack actually coming
from the main thread. This way we can put the dialog up even if the
compositor is responsive but the main thread isn't.

BUG=654835

Committed: https://crrev.com/120e997fc114e87d1b98a32ac9a81bce954e05d2
Cr-Commit-Position: refs/heads/master@{#430355}
==========

[commit-bot: I haz the power, 2016-11-07 20:22:15]

Patchset 4 (id:??) landed as
https://crrev.com/120e997fc114e87d1b98a32ac9a81bce954e05d2
Cr-Commit-Position: refs/heads/master@{#430355}