Tracking push events for lucid sleep

extensions/browser/extension_host.cc

Index: extensions/browser/extension_host.cc
diff --git a/extensions/browser/extension_host.cc b/extensions/browser/extension_host.cc
index e4bc35e0b64edbd8c6723e991b56a9b30c198797..3f1d942ee8fcd00479c811529295312b2792ac87 100644
--- a/extensions/browser/extension_host.cc
+++ b/extensions/browser/extension_host.cc
@@ -29,6 +29,7 @@
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_error.h"
 #include "extensions/browser/extension_host_delegate.h"
+#include "extensions/browser/extension_host_observer.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/extensions_browser_client.h"
 #include "extensions/browser/notification_types.h"
@@ -159,6 +160,8 @@ ExtensionHost::~ExtensionHost() {
       extensions::NOTIFICATION_EXTENSION_HOST_DESTROYED,
       content::Source<BrowserContext>(browser_context_),
       content::Details<ExtensionHost>(this));
+  FOR_EACH_OBSERVER(ExtensionHostObserver, observer_list_,
+                    OnExtensionHostDestroyed(this));
   ProcessCreationQueue::GetInstance()->Remove(this);
 }
 
@@ -205,6 +208,30 @@ void ExtensionHost::CreateRenderViewNow() {
   }
 }
 
+void ExtensionHost::AddObserver(ExtensionHostObserver* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void ExtensionHost::RemoveObserver(ExtensionHostObserver* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
+void ExtensionHost::OnMessageDispatched(const std::string& event_name,
+                                        int message_id) {
+  FOR_EACH_OBSERVER(ExtensionHostObserver, observer_list_,
+                    OnExtensionMessageDispatched(this, event_name, message_id));
+}
+
+void ExtensionHost::OnNetworkRequestStarted(uint64 request_id) {
+  FOR_EACH_OBSERVER(ExtensionHostObserver, observer_list_,
+                    OnNetworkRequestStarted(this, request_id));
+}
+
+void ExtensionHost::OnNetworkRequestDone(uint64 request_id) {
+  FOR_EACH_OBSERVER(ExtensionHostObserver, observer_list_,
+                    OnNetworkRequestDone(this, request_id));
+}
+
 const GURL& ExtensionHost::GetURL() const {
   return host_contents()->GetURL();
 }
@@ -353,10 +380,13 @@ void ExtensionHost::OnRequest(const ExtensionHostMsg_Request_Params& params) {
   extension_function_dispatcher_.Dispatch(params, render_view_host());
 }
 
-void ExtensionHost::OnEventAck() {
+void ExtensionHost::OnEventAck(int message_id) {
   EventRouter* router = EventRouter::Get(browser_context_);
   if (router)
     router->OnEventAck(browser_context_, extension_id());
+
+  FOR_EACH_OBSERVER(ExtensionHostObserver, observer_list_,

[jln (very slow on Chromium), 2015/01/15 20:19:11]

Would there be a way to validate |message_id| here?

It's very easy to forget that it's "untrusted" down the stack.
hrome/browser/chromeos/power/extension_event_observer.cc doesn't even have a
comment on that.

[Chirantan Ekbote, 2015/01/15 21:53:27]

I'm a little unclear on what you mean by validating |message_id|.  Is the
concern that a compromised renderer could start sending back bogus message ids? 
One thing I can think of is to keep a list of all unacked message ids dispatched
for the current ExtensionHost and only call the observer methods if the acked
message id exists in that list.  However, a compromised renderer could still get
lucky and send back an id that _is_ in the list and I'm not sure how we could
detect that.

[Chirantan Ekbote, 2015/01/20 23:55:43]

Ping?  I want to land this soon and I don't want to make a change just for the
sake of making a change.  I'd like to understand your concern so that I can
properly address it.

[jln (very slow on Chromium), 2015/01/22 00:16:35]

As you noted, a bogus renderer could start sending back bogus message ids. This
could cause many different types of security issues, from memory corruption to
more subtle, logical bugs.

It's not really possible for a security reviewing to go down 20 calls down the
stack to figure out why this is safe. Even if it was possible, it wouldn't be
robust as this could change at any time and it would be a time sync every time
someone looks at this.

Could you explain what the security model looks like (ideally as code comments)?
- Why is there no risk of memory corruption? Perhaps your suggestion of keeping
a list of unacked message ids is the right solution here. Or is there another
way to convince ourselves (without going 20 functions deeps) that sending a
random message_id won't cause trouble?

- What is the risk if a compromised renderer sends back a message_id that is in
the list? Can this interfere with other renderers? I don't have enough knowledge
of this area and I'm relying on your to explain this. Perhaps the risk is
acceptable, but we need to make that decision and document it.

+                    OnExtensionMessageAcked(this, message_id));
 }
 
 void ExtensionHost::OnIncrementLazyKeepaliveCount() {