[M74][Extensions] Gracefully handle unknown ids in ExtensionHost::OnEventAck

ExtensionHost::OnEventAck receives a message from a renderer that it
received and dispatched an event to the extension. Currently, we kill
the renderer (for sending a bad message) if it sends an event ID that
wasn't recorded. However, it seems there are some cases in which this
can happen even if the renderer isn't compromised or corrupted; some
hypotheses for these are described in the linked bug.

For now, since we are seeing evidence of this happening when the
renderer shouldn't be killed, relax the kill to instead be a graceful
early-out. Keep the other bad message kill for sending from a
non-background page, since that should still never happen.

TBR=rdevlin.cronin@chromium.org

(cherry picked from commit a81c4ea4d763da0f99bb0762fe5b6e6191feea45)

Bug: 939279
Change-Id: I93cb34a1a67c1a3575c5fda0b5c90b9d4f9da6f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1509363
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#638885}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1518464
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3729@{#49}
Cr-Branched-From: d4a8972-refs/heads/master@{#638880}

Author: rdcronin

extensions/browser/extension_host.cc

@@ -336,24 +336,30 @@ void ExtensionHost::OnEventAck(int event_id) {
   // events for other extensions have been acked.  Make sure that the event id
   // sent by the renderer is one that this ExtensionHost expects to receive.
   // This way if a renderer _is_ compromised, it can really only affect itself.
-  const auto it = unacked_messages_.find(event_id);
-  if (!is_background_page || it == unacked_messages_.end()) {
+  if (!is_background_page) {
     // Kill this renderer.
     DCHECK(render_process_host());
-    if (!is_background_page) {
-      LOG(ERROR) << "Killing renderer for extension " << extension_id()
-                 << " for sending an EventAck without a lazy background page.";
-    } else {
-      // We have received an unexpected event id from the renderer.  It might
-      // be compromised or it might have some other issue.
-      LOG(ERROR) << "Killing renderer for extension " << extension_id()
-                 << " for sending an EventAck message with a bad event id.";
-    }
+    LOG(ERROR) << "Killing renderer for extension " << extension_id()
+               << " for sending an EventAck without a lazy background page.";
     bad_message::ReceivedBadMessage(render_process_host(),
                                     bad_message::EH_BAD_EVENT_ID);
     return;
   }
 
+  const auto it = unacked_messages_.find(event_id);
+  if (it == unacked_messages_.end()) {
+    // Ideally, we'd be able to kill the renderer in the case of it sending an
+    // ack for an event that we haven't seen. However, https://crbug.com/939279
+    // demonstrates that there are cases in which this can happen in other
+    // situations. We should track those down and fix them, but for now
+    // log and gracefully exit.
+    // bad_message::ReceivedBadMessage(render_process_host(),
+    //                                 bad_message::EH_BAD_EVENT_ID);
+    LOG(ERROR) << "Received EventAck for extension " << extension_id()
+               << " for an unknown event.";
+    return;
+  }
+
   EventRouter* router = EventRouter::Get(browser_context_);
   if (router)
     router->OnEventAck(browser_context_, extension_id(), it->second);