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

extensions/browser/event_router.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "extensions/browser/event_router.h"
 
 #include <utility>
 
 #include "base/atomic_sequence_num.h"
 #include "base/bind.h"
@@ skipping 137 matching lines @@
 // static
 void EventRouter::DispatchEvent(IPC::Sender* ipc_sender,
                                 void* browser_context_id,
                                 const std::string& extension_id,
                                 const std::string& event_name,
                                 scoped_ptr<ListValue> event_args,
                                 UserGestureState user_gesture,
                                 const EventFilteringInfo& info) {
   int event_id = g_extension_event_id.GetNext();
 
+  IncrementInFlightEventsOnUI(browser_context_id, extension_id, event_id,
+                              event_name);
+
   DispatchExtensionMessage(ipc_sender, browser_context_id, extension_id,
                            event_id, event_name, event_args.get(), user_gesture,
                            info);
-
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(&EventRouter::IncrementInFlightEventsOnUI, browser_context_id,
-                 extension_id, event_id, event_name));
 }
 
 EventRouter::EventRouter(BrowserContext* browser_context,
                          ExtensionPrefs* extension_prefs)
     : browser_context_(browser_context),
       extension_prefs_(extension_prefs),
       extension_registry_observer_(this),
       listeners_(this) {
   registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
                  content::NotificationService::AllSources());
@@ skipping 481 matching lines @@
   }
 
   return false;
 }
 
 // static
 void EventRouter::IncrementInFlightEventsOnUI(void* browser_context_id,
                                               const std::string& extension_id,
                                               int event_id,
                                               const std::string& event_name) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {

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

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]

I've moved this check to call site.
Put the checkup at the
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.

+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&EventRouter::IncrementInFlightEventsOnUI,
+                   browser_context_id, extension_id, event_id, event_name));
+    return;
+  }
+
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   BrowserContext* browser_context =
       reinterpret_cast<BrowserContext*>(browser_context_id);
   if (!ExtensionsBrowserClient::Get()->IsValidContext(browser_context))
     return;
   EventRouter* event_router = EventRouter::Get(browser_context);
   if (!event_router)
     return;
   const Extension* extension =
       ExtensionRegistry::Get(browser_context)->enabled_extensions().GetByID(
@@ skipping 149 matching lines @@
                                      const std::string& extension_id,
                                      const GURL& listener_url,
                                      content::BrowserContext* browser_context)
     : event_name(event_name),
       extension_id(extension_id),
       listener_url(listener_url),
       browser_context(browser_context) {
 }
 
 }  // namespace extensions