Fix race condition causing DCHECK(ack_pending_) to trip.

Fix race condition causing DCHECK(ack_pending_) to trip.

If RenderFrameImpl deletes a RenderAccessibilityImpl and then recreates
another one very quickly, that second RenderAccessibilityImpl can get
confused when it receives an ACK intended for the previous instance.

Fix this by passing an ACK token from the renderer to browser and ignoring
messages with the wrong token.

This only caused issues when DCHECKs are enabled.

BUG=646588
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/50aa111be5288fbc06b54e09d2cafd4dd294a315
Cr-Commit-Position: refs/heads/master@{#418902}

[dmazzoni, 2016-09-13 21:09:35]

Description was changed from

==========
Fix race condition causing DCHECK(ack_pending_) to trip.

If RenderFrameImpl deletes a RenderAccessibilityImpl and then recreates
another one very quickly, that second RenderAccessibilityImpl can get
confused when it receives an ACK intended for the previous instance.

Fix this by passing an ACK token from the renderer to browser and ignoring
messages with the wrong token.

This only caused issues when DCHECKs are enabled.

BUG=646588
==========

to

==========
Fix race condition causing DCHECK(ack_pending_) to trip.

If RenderFrameImpl deletes a RenderAccessibilityImpl and then recreates
another one very quickly, that second RenderAccessibilityImpl can get
confused when it receives an ACK intended for the previous instance.

Fix this by passing an ACK token from the renderer to browser and ignoring
messages with the wrong token.

This only caused issues when DCHECKs are enabled.

BUG=646588
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[dmazzoni, 2016-09-13 21:09:44]

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

[dmazzoni, 2016-09-13 21:09:54]

dmazzoni@chromium.org changed reviewers:
+ dtseng@chromium.org, tsepez@chromium.org

[commit-bot: I haz the power, 2016-09-13 21:10:17]

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-09-13 21:12:54]

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

[commit-bot: I haz the power, 2016-09-13 21:12:55]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Tom Sepez, 2016-09-13 21:31:49]

Messages LGTM with nit but bots are red.

https://codereview.chromium.org/2341503002/diff/1/content/common/accessibilit...
File content/common/accessibility_messages.h (right):

https://codereview.chromium.org/2341503002/diff/1/content/common/accessibilit...
content/common/accessibility_messages.h:184: // message was processed and it can
send addition events. The argument
nit: additional

[David Tseng, 2016-09-14 02:49:42]

lgtm

https://codereview.chromium.org/2341503002/diff/1/content/renderer/accessibil...
File content/renderer/accessibility/render_accessibility_impl.cc (right):

https://codereview.chromium.org/2341503002/diff/1/content/renderer/accessibil...
content/renderer/accessibility/render_accessibility_impl.cc:88: ack_token_ =
g_next_ack_token++;
Perhaps dumb question, but is this global shared across processes?

[dmazzoni, 2016-09-14 18:58:04]

https://codereview.chromium.org/2341503002/diff/1/content/renderer/accessibil...
File content/renderer/accessibility/render_accessibility_impl.cc (right):

https://codereview.chromium.org/2341503002/diff/1/content/renderer/accessibil...
content/renderer/accessibility/render_accessibility_impl.cc:88: ack_token_ =
g_next_ack_token++;
On 2016/09/14 at 02:49:42, David Tseng wrote:
> Perhaps dumb question, but is this global shared across processes?

No, but it doesn't need to be. The only issue is when the same RenderFrameHost
deletes and creates a new RenderAccessibilityImpl, so it only needs to be global
within this render process.

[dmazzoni, 2016-09-14 18:59:26]

Bots were red due to patch failure. Should be ready to go if CQ passes, thanks.

https://codereview.chromium.org/2341503002/diff/1/content/common/accessibilit...
File content/common/accessibility_messages.h (right):

https://codereview.chromium.org/2341503002/diff/1/content/common/accessibilit...
content/common/accessibility_messages.h:184: // message was processed and it can
send addition events. The argument
On 2016/09/13 at 21:31:49, Tom Sepez wrote:
> nit: additional

Fixed

[dmazzoni, 2016-09-14 18:59:38]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-09-14 18:59:38]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
dtseng@chromium.org
Link to the patchset: https://codereview.chromium.org/2341503002/#ps40001
(title: "Fix typo")

[commit-bot: I haz the power, 2016-09-14 19:00:33]

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-09-14 19:04:29]

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

[commit-bot: I haz the power, 2016-09-14 19:04:30]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dmazzoni, 2016-09-14 20:36:50]

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

[commit-bot: I haz the power, 2016-09-14 20:37:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dmazzoni, 2016-09-14 20:37:32]

dmazzoni@chromium.org changed reviewers:
+ jam@chromium.org

[dmazzoni, 2016-09-14 20:37:33]

+jam for content/browser/frame_host

[commit-bot: I haz the power, 2016-09-14 20:56:19]

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

[commit-bot: I haz the power, 2016-09-14 20:56:20]

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

[dmazzoni, 2016-09-14 21:32:18]

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

[commit-bot: I haz the power, 2016-09-14 21:33:44]

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-09-14 22:39:02]

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

[commit-bot: I haz the power, 2016-09-14 22:39:06]

Dry run: This issue passed the CQ dry run.

[jam, 2016-09-15 16:24:40]

lgtm

[dmazzoni, 2016-09-15 17:52:50]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-09-15 17:52:51]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
dtseng@chromium.org
Link to the patchset: https://codereview.chromium.org/2341503002/#ps80001
(title: "Update test")

[commit-bot: I haz the power, 2016-09-15 17:53: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, 2016-09-15 17:59:29]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-15 18:01:04]

Description was changed from

==========
Fix race condition causing DCHECK(ack_pending_) to trip.

If RenderFrameImpl deletes a RenderAccessibilityImpl and then recreates
another one very quickly, that second RenderAccessibilityImpl can get
confused when it receives an ACK intended for the previous instance.

Fix this by passing an ACK token from the renderer to browser and ignoring
messages with the wrong token.

This only caused issues when DCHECKs are enabled.

BUG=646588
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Fix race condition causing DCHECK(ack_pending_) to trip.

If RenderFrameImpl deletes a RenderAccessibilityImpl and then recreates
another one very quickly, that second RenderAccessibilityImpl can get
confused when it receives an ACK intended for the previous instance.

Fix this by passing an ACK token from the renderer to browser and ignoring
messages with the wrong token.

This only caused issues when DCHECKs are enabled.

BUG=646588
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/50aa111be5288fbc06b54e09d2cafd4dd294a315
Cr-Commit-Position: refs/heads/master@{#418902}
==========

[commit-bot: I haz the power, 2016-09-15 18:01:05]

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