[HBD] Remove PluginCache and reload plugin list when origin changes

content/browser/frame_host/render_frame_message_filter.cc

Index: content/browser/frame_host/render_frame_message_filter.cc
diff --git a/content/browser/frame_host/render_frame_message_filter.cc b/content/browser/frame_host/render_frame_message_filter.cc
index a4a91b09e4869065e1ba2ea995e6442ea2a7d189..22bdf086cd8a068b005fcc2063b8244910d147fd 100644
--- a/content/browser/frame_host/render_frame_message_filter.cc
+++ b/content/browser/frame_host/render_frame_message_filter.cc
@@ -466,9 +466,9 @@ void RenderFrameMessageFilter::SetCookie(int32_t render_frame_id,
 
 #if defined(ENABLE_PLUGINS)
 
-void RenderFrameMessageFilter::OnGetPlugins(
-    bool refresh,
-    IPC::Message* reply_msg) {
+void RenderFrameMessageFilter::OnGetPlugins(bool refresh,
+                                            url::Origin main_frame_origin,
+                                            IPC::Message* reply_msg) {
   // Don't refresh if the specified threshold has not been passed.  Note that
   // this check is performed before off-loading to the file thread.  The reason
   // we do this is that some pages tend to request that the list of plugins be
@@ -486,12 +486,14 @@ void RenderFrameMessageFilter::OnGetPlugins(
     }
   }
 
-  PluginServiceImpl::GetInstance()->GetPlugins(base::Bind(
-      &RenderFrameMessageFilter::GetPluginsCallback, this, reply_msg));
+  PluginServiceImpl::GetInstance()->GetPlugins(
+      base::Bind(&RenderFrameMessageFilter::GetPluginsCallback, this, reply_msg,
+                 main_frame_origin));
 }
 
 void RenderFrameMessageFilter::GetPluginsCallback(
     IPC::Message* reply_msg,
+    url::Origin main_frame_origin,
     const std::vector<WebPluginInfo>& all_plugins) {
   // Filter the plugin list.
   PluginServiceFilter* filter = PluginServiceImpl::GetInstance()->GetFilter();
@@ -502,12 +504,10 @@ void RenderFrameMessageFilter::GetPluginsCallback(
   // In this loop, copy the WebPluginInfo (and do not use a reference) because
   // the filter might mutate it.
   for (WebPluginInfo plugin : all_plugins) {
-    if (!filter || filter->IsPluginAvailable(child_process_id,
-                                             routing_id,
-                                             resource_context_,
-                                             GURL(),
-                                             GURL(),
-                                             &plugin)) {
+    if (!filter ||
+        filter->IsPluginAvailable(

[alexmos, 2016/08/02 22:06:27]

Looking at IsPluginAvailable implementations, they don't use the policy_url
param that's populated here atm - is that going to come in another CL?

[trizzofo, 2016/08/03 00:08:37]

Yes, we are working on it:
https://crrev.com/2208463002

+            child_process_id, routing_id, resource_context_, GURL(),
+            GURL(main_frame_origin.Serialize()), &plugin)) {

[alexmos, 2016/08/02 22:06:27]

Is there a way to just use url::Origins in IsPluginAvailable as well?  It'd be
nice to minimize these kinds of origin conversions when possible, as they might
lose information.  In particular, a GURL doesn't track uniqueness: a unique
origin can't access another unique origin, but a GURL created from one unique
origin (serialized as "null") would be invalid and would be considered equal to
a GURL created from another unique origin.  So you'd need to be very careful in
comparing them.

[trizzofo, 2016/08/03 00:08:37]

The only reason why we're using unique origins is because it's empty, so, when
it reaches isPluginAvailable(), the default content setting is used. For
readability reasons, tommycli suggested changing it to a non-unique empty
origin. If we change it, would it still make sense to use url::Origin instead of
GURL inside isPluginAvailable()?

[alexmos, 2016/08/04 05:32:02]

Ah, looking at your https://crrev.com/2208463002, it looks like you'll end up
passing this origin into content settings, which means you'll have to convert
into a GURL anyway, at least until
https://bugs.chromium.org/p/chromium/issues/detail?id=621724 is fixed so that
content settings support url::Origins.  Looking a bit more at isPluginAvailable,
it seems that making it take url::Origins will touch a bunch more unrelated
places, so I'm fine leaving this for a future CL, for whoever is going to work
on 621724.

What do you mean by a non-unique, empty origin?  Looking at url::Origin's
constructor, it will default to a unique origin unless the scheme/host/port are
actually valid.  (FWIW, I think it's fine to use a unique origin to get to
default content settings - I think that's also done in some other content
settings lookups.)

[trizzofo, 2016/08/05 02:04:25]

Yes, I took a better look at url::Origin constructors and you're right. In that
case, we should probably leave it as an unique url::Origin.

       plugins.push_back(plugin);
     }
   }