Fix a race where backgroud page might ack an event before we're ready

Fix a race where backgroud page might ack an event before we're ready
    to receive the ack on browser/.

We perform a sanity check upon background page ACKing an event,
    by checking whether the ACKed message is expected to
    be ACKed. We store the pending ACK message IDs into a set
    |unacked_messages_|, but due to PostTask call to
    IncrementInFlightEventsOnUI, the event ack might arrive earlier
    than storing the pending message's ID. This results in incorectly
    kill the renderer process.

BUG=485440, 473821
Test=Rise Vision chrome app shouldn't crash anymore.

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

[lazyboy, 2015-05-15 21:13:56]

lazyboy@chromium.org changed reviewers:
+ chirantan@chromium.org, kalman@chromium.org

[not at google - send to devlin, 2015-05-15 21:18:07]

https://codereview.chromium.org/1138643010/diff/1/extensions/browser/event_ro...
File extensions/browser/event_router.cc (right):

https://codereview.chromium.org/1138643010/diff/1/extensions/browser/event_ro...
extensions/browser/event_router.cc:665: if
(!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
For a function called "...OnUI" this line is misleading. Put the checkup at the
call site, so that it's obvious - and we should figure out what to do in the
case where it's *not* on the UI thread. We can't just ignore that?

[lazyboy, 2015-05-15 21:29:53]

https://codereview.chromium.org/1138643010/diff/1/extensions/browser/event_ro...
File extensions/browser/event_router.cc (right):

https://codereview.chromium.org/1138643010/diff/1/extensions/browser/event_ro...
extensions/browser/event_router.cc:665: if
(!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
On 2015/05/15 21:18:07, kalman wrote:
> For a function called "...OnUI" this line is misleading.
I've moved this check to call site.
Put the checkup at the
> call site, so that it's obvious - and we should figure out what to do in the
> case where it's *not* on the UI thread. We can't just ignore that?
For non-UI thread, I'm still PostTasking to UI to keep the behavior same in this
CL. I need to investigate more on the keep alive semantics before I'm
comfortable /w ignoring Increment... call on non-UI.
This means this change currently fixes race only when ::DispatchEvent() is
called from UI thread.

[not at google - send to devlin, 2015-05-15 21:31:11]

Ok, sgtm.

[lazyboy, 2015-05-15 21:53:38]

Well, my git cl upload hung, now uploaded. Patch Set #2.

[not at google - send to devlin, 2015-05-15 21:55:29]

lgtm

https://chromiumcodereview.appspot.com/1138643010/diff/20001/extensions/brows...
File extensions/browser/event_router.cc (right):

https://chromiumcodereview.appspot.com/1138643010/diff/20001/extensions/brows...
extensions/browser/event_router.cc:159: BrowserThread::PostTask(
comment when this is the case, what happens in that case, what the problem is,
and what you're planning on doing about it (maybe just a bug link).

[lazyboy, 2015-05-16 00:07:43]

https://chromiumcodereview.appspot.com/1138643010/diff/20001/extensions/brows...
File extensions/browser/event_router.cc (right):

https://chromiumcodereview.appspot.com/1138643010/diff/20001/extensions/brows...
extensions/browser/event_router.cc:159: BrowserThread::PostTask(
On 2015/05/15 21:55:29, kalman wrote:
> comment when this is the case, what happens in that case, what the problem is,
> and what you're planning on doing about it (maybe just a bug link).

Done.

[lazyboy, 2015-05-16 03:00:30]

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

[lazyboy, 2015-05-16 03:00:30]

The patchset sent to the CQ was uploaded after l-g-t-m from kalman@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1138643010/#ps40001
(title: "address comments")

[commit-bot: I haz the power, 2015-05-16 03:00:41]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1138643010/40001

[commit-bot: I haz the power, 2015-05-16 03:51:18]

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

[commit-bot: I haz the power, 2015-05-16 03:51:19]

Dry run: This issue passed the CQ dry run.

[Chirantan Ekbote, 2015-05-16 18:06:46]

lgtm

[lazyboy, 2015-05-17 04:39:25]

The CQ bit was checked by lazyboy@chromium.org

[commit-bot: I haz the power, 2015-05-17 04:39:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1138643010/40001

[commit-bot: I haz the power, 2015-05-17 06:15:36]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-05-18 11:32:30]

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