Service worker bypass for resource requests from suborigins

third_party/WebKit/Source/core/loader/FrameFetchContext.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 576 matching lines @@
     DocumentLoader* documentLoader = masterDocumentLoader();
     if (documentLoader && documentLoader->subresourceFilter() && type != Resource::MainResource && type != Resource::ImportResource && !documentLoader->subresourceFilter()->allowLoad(url, resourceRequest.requestContext()))
         return ResourceRequestBlockedReasonSubresourceFilter;
 
     return ResourceRequestBlockedReasonNone;
 }
 
 bool FrameFetchContext::isControlledByServiceWorker() const
 {
     ASSERT(m_documentLoader || frame()->loader().documentLoader());
+
+    if (getSecurityOrigin() && getSecurityOrigin()->hasSuborigin())
+        return false;

[falken, 2016/09/20 03:48:57]

Why is this needed?

I'm a little concerned that the document will have a
navigator.serviceWorker.controller, but all requests must skip the service
worker. I'm hoping the skip_service_worker flag is robust enough to handle this
state (we've previously had issues around CORS logic when the renderer's
expectations of whether SW touches a request are violated). It might be cleaner
to just not have a navigator.serviceWorker.controller but doing that is also
really tricky because we set it when processing the navigation request in the
browser process, before the response comes in. And we'd also have to ensure SW
can't later claim() the document.

Not considering implementation difficulty, is the desired behavior to have a
.controller when on a suborigin?

[jww, 2016/09/20 04:27:47]

No, and in fact in a previous CL, we disabled the JS API to
navigator.serviceWorker. See
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/http/test...

From my perspective, I'd rather stick with easier implementation perspective for
now, and deal with any bugs that come up (which, I think, you're saying is to
use skip_service_worker), and then as suborigins become more robust, we go one
of two routes:
   * Completely remove the controller.
   * If a future version of the spec allows for service workers on suborigins,
implement a correct controller behavior.

Does that make sense as an approach?

[falken, 2016/09/20 04:52:52]

Makes sense to me. Let's go with this and see.

BTW disabling the JS API doesn't totally prevent registering a SW. Do you also
stop Link rel=serviceworker
https://www.chromestatus.com/feature/5682681044008960?

+
     if (m_documentLoader)
         return frame()->loader().client()->isControlledByServiceWorker(*m_documentLoader);
     // m_documentLoader is null while loading resources from an HTML import.
     // In such cases whether the request is controlled by ServiceWorker or not
     // is determined by the document loader of the frame.
     return frame()->loader().client()->isControlledByServiceWorker(*frame()->loader().documentLoader());
 }
 
 int64_t FrameFetchContext::serviceWorkerID() const
 {
@@ skipping 192 matching lines @@
 }
 
 DEFINE_TRACE(FrameFetchContext)
 {
     visitor->trace(m_document);
     visitor->trace(m_documentLoader);
     FetchContext::trace(visitor);
 }
 
 } // namespace blink