service worker: Don't control a subframe of an insecure context

third_party/WebKit/public/web/WebFrame.h

Index: third_party/WebKit/public/web/WebFrame.h
diff --git a/third_party/WebKit/public/web/WebFrame.h b/third_party/WebKit/public/web/WebFrame.h
index c1245a3a29cbdadc5f4acae6e8bfe43e48bee058..a4533bec18339b6dc1077cb9c2f17cc0c5ce0c13 100644
--- a/third_party/WebKit/public/web/WebFrame.h
+++ b/third_party/WebKit/public/web/WebFrame.h
@@ -552,6 +552,7 @@ public:
     // Returns the frame inside a given frame or iframe element. Returns 0 if
     // the given element is not a frame, iframe or if the frame is empty.
     BLINK_EXPORT static WebFrame* fromFrameOwnerElement(const WebElement&);
+    BLINK_EXPORT bool canHaveSecureChild(WebString& errorMessage) const;

[alexmos, 2016/05/26 22:13:35]

Do we need both this and WebDocument::isSecureContext in the public API?  Seems
WebDocument::isSecureContext is only used in plugins, where it's called on the
plugin container frame.  Could that just use canHaveSecureChild?

Also, since both uses don't care about the error message, maybe we don't need it
in the public API either?

[falken, 2016/05/27 05:47:04]

I think there's two problems there:
1) isSecureContext allows for scheme exceptions and canHaveSecureChild doesn't.
2) A Document can be frameless (apparently. the comments for WebDocument::frame
say so). So document.isSecureContext() can't be transformed to
document.frame()->canHaveSecureChild().

1) could probably be resolved by refactoring to a common method, but 2) looks
more difficult.

To be more specific, IIRC, the plugin situation looks like:
- Frame
  - Document
    ...
      - plugin container (Element)
        - plugin instance

It's calling isSecureContext on the document. So it can be whitelisted by
scheme. If we instead call Frame::canHaveSecureChild, the scheme whitelist
doesn't happen. And we might not have a frame.

Probably canHaveSecureChild should be renamed to something like
canHaveSecureChildFrameIgnoringSchemeExceptions.

Having said all that, the plugin check is just for metrics and isn't a security
check. So maybe it's ok being slightly "wrong" there? +raymes
I can certainly remove it.

[alexmos, 2016/05/27 22:44:30]

Yeah, I'm curious if the plugin case cares about scheme exceptions at all. 
Looks like it was added (by raymes@ in r329895) before scheme exceptions were
introduced (by jww@ in r354650), so I'll let jww@ and raymes@ comment. :)

[raymes, 2016/05/30 01:08:08]

The plugin case is just for metrics - we were just testing how frequently
plugins were loaded in secure contexts (that is, it's src is https and it's
parents are https). If we get something which approximates that it should be
fine especially if it simplifies code.

jww: Do we even still need those metrics? timwillis may know.

The metrics are: Pepper.PluginContextSecurity.TCPConnect and
Pepper.PluginContextSecurity.UDPBind

 
 #if BLINK_IMPLEMENTATION
     static WebFrame* fromFrame(Frame*);