Refactor: cleanup declarative_api.cc.

extensions/renderer/event_bindings.cc

 // Copyright 2014 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/renderer/event_bindings.h"
 
 #include <map>
 #include <set>
 #include <string>
 #include <vector>
@@ skipping 147 matching lines @@
   RouteFunction("MatchAgainstEventFilter",
                 base::Bind(&EventBindings::MatchAgainstEventFilter,
                            base::Unretained(this)));
 }
 
 EventBindings::~EventBindings() {}
 
 // Attach an event name to an object.
 void EventBindings::AttachEvent(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
-  CHECK_EQ(1, args.Length());
+  CHECK_EQ(2, args.Length());
   CHECK(args[0]->IsString());
+  CHECK(args[1]->IsInt32());
 
   std::string event_name = *v8::String::Utf8Value(args[0]);
+  int guest_view_instance_id = args[1]->Int32Value();
 
-  if (!dispatcher_->CheckContextAccessToExtensionAPI(event_name, context()))
+  if (!guest_view_instance_id &&

[not at google - send to devlin, 2014/12/11 22:10:48]

If it's an int, prefer to use guest_view_instance_id == 0 rather than a boolean
expression.

[Xi Han, 2014/12/12 22:28:30]

Done.

+      !dispatcher_->CheckContextAccessToExtensionAPI(event_name, context()))

[not at google - send to devlin, 2014/12/11 22:10:48]

It's nice to surround this in {} when the condition is complex (i.e.
multi-line).

[Xi Han, 2014/12/12 22:28:30]

Done.

     return;
 

[not at google - send to devlin, 2014/12/11 22:10:48]

All the above said, it's pretty weird to see security checks being skipped here
just because it's from a webview. Could you modify the event functions to be
callable from web contexts, then you don't need this change?

[Xi Han, 2014/12/12 22:28:30]

As discussed offline, the problem here is: if passing in the event with its
original name like "declarativeWebRequest.onMessage", this event will be treated
as a regular API event, go through the permission check, and won't pass the
feature permission check here. So we do need to give <webview> event some power
to skip the check. 

   const std::string& extension_id = context()->GetExtensionID();
   if (IncrementEventListenerCount(context(), event_name) == 1) {
     content::RenderThread::Get()->Send(new ExtensionHostMsg_AddListener(
         extension_id, context()->GetURL(), event_name));
   }
 
   // This is called the first time the page has added a listener. Since
   // the background page is the only lazy page, we know this is the first
   // time this listener has been registered.
   if (IsLazyBackgroundPage(context()->GetRenderView(),
@@ skipping 132 matching lines @@
 }
 
 scoped_ptr<EventMatcher> EventBindings::ParseEventMatcher(
     base::DictionaryValue* filter_dict) {
   return scoped_ptr<EventMatcher>(new EventMatcher(
       scoped_ptr<base::DictionaryValue>(filter_dict->DeepCopy()),
       context()->GetRenderView()->GetRoutingID()));
 }
 
 }  // namespace extensions