Plugin Power Saver: Restrict Power Saver to cross-origin.

content/renderer/pepper/pepper_plugin_instance_impl.cc

Index: content/renderer/pepper/pepper_plugin_instance_impl.cc
diff --git a/content/renderer/pepper/pepper_plugin_instance_impl.cc b/content/renderer/pepper/pepper_plugin_instance_impl.cc
index 97a85c90f253966dd8e964bd7232cfe7d199e90f..511e3f894c43fc68d5387fa4aace30115ab8f811 100644
--- a/content/renderer/pepper/pepper_plugin_instance_impl.cc
+++ b/content/renderer/pepper/pepper_plugin_instance_impl.cc
@@ -583,13 +583,9 @@ PepperPluginInstanceImpl::PepperPluginInstanceImpl(
       GetContentClient()->renderer()->IsExternalPepperPlugin(module->name()))
     external_document_load_ = true;
 
-  // TODO(tommycli): Insert heuristics to determine whether plugin content
-  // is peripheral here.
-  bool is_peripheral_content = true;
-  power_saver_enabled_ = is_peripheral_content &&
-      module->name() == kFlashPluginName &&
-      CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnablePluginPowerSaver);
+  power_saver_enabled_ = CommandLine::ForCurrentProcess()->HasSwitch(
+                             switches::kEnablePluginPowerSaver) &&
+                         IsPeripheralContent();
 
   if (power_saver_enabled_) {
     throttler_.reset(new PepperPluginInstanceThrottler(
@@ -3299,6 +3295,14 @@ void PepperPluginInstanceImpl::DidDataFromWebURLResponse(
   }
 }
 
+bool PepperPluginInstanceImpl::IsPeripheralContent() const {
+  if (module_->name() != kFlashPluginName)
+    return false;
+
+  GURL top_frame_url = render_frame_->GetWebFrame()->top()->document().url();

[groby-ooo-7-16, 2014/10/23 00:53:45]

Using GetContainer()->element().document().url() would simplify even more :)

(Sorry, I didn't know _that_ path was available either, but see raymes comment
above)

[raymes, 2014/10/23 01:34:07]

Are you sure you want the main frame?

Consider the following case:
I have a site that contains an iframe which contains a plugin. Which URL do you
want to compare the plugin URL to? I think it makes more sense to compare to the
URL which the plugin is actually embedded in.

[groby-ooo-7-16, 2014/10/23 01:59:13]

That'd be the iframe's URL, right? We definitely don't want that.  I.e if
mydomain.com includes flash _hosted_ on mydomain.com, we consider that not
peripheral, no matter if iframed or not.

[raymes, 2014/10/23 03:53:38]

I was thinking about a situation where origin X has an iframe at origin Y which
embeds a plugin at origin Y. In that case, the origin of the plugin (Y) would
not match the top level frame (X) so it would be considered peripheral, even
though it may be the main focus of the website. Imagine a site which contains an
iframe which is navigated to youtube. Is this the expected behavior?

I may be completely missing something so sorry if this sounds wrong!

[piman, 2014/10/23 03:56:32]

If we're in an out-of-process iframe, I think this will crash
(WebRemoteFrameImpl::document() will return an empty WebDocument with a NULL
wrapped Document, and calling url() on it will crash).

+creis I'm not sure if this is supposed to eventually work or not (I'm assuming
a frame could know the URL of its parent since a navigation in the parent would
remove the child iframe, but I may be missing edge cases).

[groby-ooo-7-16, 2014/10/23 05:51:19]

Will GetContainer()->element().document().url( still work in the OOPIF case?

[groby-ooo-7-16, 2014/10/23 05:51:19]

This is indeed expected behavior - this CL is not the complete behavior. As long
as the plugin at origin Y exceeds certain size metrics, it will be still
relevant. The current CL just catches the case of small helper-plugins from the
same domain.

[tommycli, 2014/10/23 17:23:27]

We need render_frame_->GetWebFrame()->top()->document().url(), as
GetContainer()->element().document().url() gets the URL of the IFRAME's source,
not the top level document.

And I can confirm that render_frame_->GetWebFrame()->top()->document().url()
does crash for cross-site iframes...

[Charlie Reis, 2014/10/23 18:23:27]

It seems like it's unclear to multiple reviewers what policy you're trying
enforce.  I'd recommend documenting the intent in a comment, even if you plan to
evolve it later.  That way we can check whether the code matches the intent.

As for OOPIF, it's guaranteed that a RemoteFrame will always be cross-origin
from a LocalFrame.  Thus, if the main frame is a RemoteFrame (i.e.,
render_frame_->GetWebFrame()->top()->isWebRemoteFrame()), we know it has a
different origin than render_frame_ and don't need to check.

Of course, that might not match your intent if you have a main frame on A, with
an iframe on B, hosting a plugin from A.  We wouldn't notice that the plugin and
the main frame come from the same origin.  If you need that, you might be able
to use Alex's upcoming origin replication for RemoteFrames, or you could do this
check in the browser process and just let the renderer know.

[tommycli, 2014/10/23 19:32:23]

I added some comments. It is indeed a confusing intent.

Any time the plugin and the top level frame have the same origin, I do need to
know, so it can be marked as essential content.

SOME cross-origin plugin content is peripheral, but ALL same-origin plugin
content is essential.

I'll almost certainly have to bounce to the browser process to handle the
cross-origin case, but I'd like to avoid IPC for plugins from the some origin as
the top level frame, as those are essential and should be played without delay.

Do you have any more info on Alex's origin replication for RemoteFrames?

[Charlie Reis, 2014/10/23 19:51:08]

Yes, see http://crbug.com/426512.  It's a fairly big change that he's working
on.

+  return plugin_url_.GetOrigin() != top_frame_url.GetOrigin();
+}
+
 void PepperPluginInstanceImpl::SetPluginThrottled(bool throttled) {
   // Do not throttle if we've already disabled power saver.
   if (!power_saver_enabled_ && throttled)