Extensions: Send the tab id to platform apps.

chrome/renderer/extensions/messaging_bindings.cc

Index: chrome/renderer/extensions/messaging_bindings.cc
===================================================================
--- chrome/renderer/extensions/messaging_bindings.cc	(revision 246091)
+++ chrome/renderer/extensions/messaging_bindings.cc	(working copy)
@@ -23,6 +23,7 @@
 #include "chrome/renderer/extensions/chrome_v8_extension.h"
 #include "chrome/renderer/extensions/dispatcher.h"
 #include "chrome/renderer/extensions/event_bindings.h"
+#include "chrome/renderer/extensions/media_galleries_custom_bindings.h"
 #include "chrome/renderer/extensions/scoped_persistent.h"
 #include "content/public/renderer/render_thread.h"
 #include "content/public/renderer/render_view.h"
@@ -47,6 +48,8 @@
 using content::RenderThread;
 using content::V8ValueConverter;
 
+namespace extensions {
+
 namespace {
 
 struct ExtensionData {
@@ -77,11 +80,10 @@
 const char kReceivingEndDoesntExistError[] =
     "Could not establish connection. Receiving end does not exist.";
 
-class ExtensionImpl : public extensions::ChromeV8Extension {
+class ExtensionImpl : public ChromeV8Extension {
  public:
-  explicit ExtensionImpl(extensions::Dispatcher* dispatcher,
-                         extensions::ChromeV8Context* context)
-      : extensions::ChromeV8Extension(dispatcher, context) {
+  explicit ExtensionImpl(Dispatcher* dispatcher, ChromeV8Context* context)
+      : ChromeV8Extension(dispatcher, context) {
     RouteFunction("CloseChannel",
         base::Bind(&ExtensionImpl::CloseChannel, base::Unretained(this)));
     RouteFunction("PortAddRef",
@@ -117,9 +119,8 @@
 
     renderview->Send(new ExtensionHostMsg_PostMessage(
         renderview->GetRoutingID(), port_id,
-        extensions::Message(
-            *v8::String::Utf8Value(args[1]),
-            blink::WebUserGestureIndicator::isProcessingUserGesture())));
+        Message(*v8::String::Utf8Value(args[1]),
+                blink::WebUserGestureIndicator::isProcessingUserGesture())));
   }
 
   // Forcefully disconnects a port.
@@ -210,8 +211,8 @@
       callback->Call(context->Global(), 0, NULL);
     }
 
-    extensions::ScopedPersistent<v8::Object> object_;
-    extensions::ScopedPersistent<v8::Function> callback_;
+    ScopedPersistent<v8::Object> object_;
+    ScopedPersistent<v8::Function> callback_;
     v8::Isolate* isolate_;
 
     DISALLOW_COPY_AND_ASSIGN(GCCallback);
@@ -232,8 +233,6 @@
 
 }  // namespace
 
-namespace extensions {
-
 ChromeV8Extension* MessagingBindings::Get(
     Dispatcher* dispatcher,
     ChromeV8Context* context) {
@@ -272,7 +271,7 @@
       continue;
 
     v8::Handle<v8::Value> tab = v8::Null(isolate);
-    if (!source_tab.empty())
+    if (!source_tab.empty() && (*it)->extension()->is_platform_app())

[not at google - send to devlin, 2014/01/24 21:37:27]

shouldn't this be !is_platform_app() ?

[Lei Zhang, 2014/01/25 01:47:09]

Whoops.

       tab = converter->ToV8Value(&source_tab, (*it)->v8_context());
 
     v8::Handle<v8::Value> tls_channel_id_value = v8::Undefined(isolate);
@@ -290,26 +289,37 @@
     }
 
     v8::Handle<v8::Value> arguments[] = {
+      // portId
       v8::Integer::New(isolate, target_port_id),
+      // channelName
       v8::String::NewFromUtf8(isolate,
                               channel_name.c_str(),
                               v8::String::kNormalString,
                               channel_name.size()),
-      tab, v8::String::NewFromUtf8(isolate,
-                                   source_extension_id.c_str(),
-                                   v8::String::kNormalString,
-                                   source_extension_id.size()),
+      // sourceTab
+      tab,
+      // sourceExtensionId
       v8::String::NewFromUtf8(isolate,
+                              source_extension_id.c_str(),
+                              v8::String::kNormalString,
+                              source_extension_id.size()),
+      // targetExtensionId
+      v8::String::NewFromUtf8(isolate,
                               target_extension_id.c_str(),
                               v8::String::kNormalString,
                               target_extension_id.size()),
+      // sourceUrl
       v8::String::NewFromUtf8(isolate,
                               source_url_spec.c_str(),
                               v8::String::kNormalString,
                               source_url_spec.size()),
+      // tlsChannelId
       tls_channel_id_value,
     };
 
+    int tab_id = -1;
+    source_tab.GetInteger("id", &tab_id);
+    MediaGalleriesCustomBindings::PushSenderTabId(*it, tab_id);

[not at google - send to devlin, 2014/01/24 21:37:27]

Having this in MediaGalleriesCustomBindings is odd since it's a generic
messaging functionality. Just put the push/pop logic in this class.

I don't understand why you can't have this scoped thing though. The function
call needs to be made synchronously during sendRequest so it will happen within
the CallModuleMethod call below. That could set a global variable somewhere,
with an implicit stack (from the scoped thing saving/restoring the existing tab
ID value) to read out of.

Sorry if I'm missing something here.

[Lei Zhang, 2014/01/25 01:47:09]

Sure, moved here.
Are you assuming the API is synchronous? Media Galleries APIs are async.

[not at google - send to devlin, 2014/01/27 17:51:46]

No, it's an async API, but running the callback itself is synchronous and that's
what matters(?):

chrome.runtime.onMessageExternal.addListener(function() {
  // user action = true, tab ID = 42
  chrome.mediaGalleries.doSomething(function() {
  });
  // user action = false, tab ID = undefined
});

i.e. the chrome.mediaGalleries.doSomething call is executed with that user
action / tab ID state because the event listener has it scoped, so if you read
it from the generic SendRequest place it will be set.

Sorry if I'm missing something.

[Lei Zhang, 2014/01/27 23:19:13]

Ah I see what you mean. I'll try to narrow the scope down and see if we can get
it to a point where we can make it scoped. If you happen to know where that is,
that would be very helpful.

[not at google - send to devlin, 2014/01/27 23:27:40]

So what I'd probably do is have that scoped thing in request_sender.h. Say,
something like:

class RequestSender {
  class ScopedTabID {
    explicit ScopedTabID(RequestSender* rs, int tab_id)
        : rs_(rs), tab_id_(tab_id) {
      old_ = rs->current_tab_id_;
      rs->current_tab_id_ = tab_id;
    }

    ~ScopedTabID() {
      DCHECK_EQ(tab_id_, rs->current_tab_id_);
      rs->current_tab_id_ = old_;
    }

    RequestSender* rs_;
    int tab_id_;
  }

  ...
  friend class ScopedTabID;
  int current_tab_id_;  // start at -1
}

then declare one of those inside the Dispatcher::DispatchEvent call? I think
it's just event dispatching that you need?

sorry rushing through this, hope that's right.

     v8::Handle<v8::Value> retval = (*it)->module_system()->CallModuleMethod(
         "messaging",
         "dispatchOnConnect",
@@ -415,6 +425,7 @@
     } else {
       arguments.push_back(v8::Null(isolate));
     }
+    MediaGalleriesCustomBindings::PopSenderTabId(*it);
     (*it)->module_system()->CallModuleMethod("messaging",
                                              "dispatchOnDisconnect",
                                              &arguments);