Add blink-side plumbing for ServiceWorker -> Document postMessage (1/3)

Add blink-side plumbing for ServiceWorker -> Document postMessage

- Initialize NavigatorServiceWorker and ServiceWorkerContainer earlier, right after a document is available (for Documents)
- Add public interfaces:
-- WebServiceWorkerContextClient.postMessageToClient()
-- WebServiceWorkerProviderClient.dispatchMessageEvent()

This is the first patch for postMessage plumbing: 
1/3: THIS PATCH
2/3: https://codereview.chromium.org/246023007/ (chromium) 
3/3: https://codereview.chromium.org/264233003/ (blink, adds Layout test)

BUG=366063
TEST=to be added after chromium-side change is landed

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=173495

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=173546

[kinuko, 2014-05-05 17:44:26]

Haven't fully wired/tested yet, but I think this can work.  PTL

[jsbell, 2014-05-05 19:03:05]

Change LGTM but it seems like one of the key files here is (already) violating
the Blink public API design.

Maybe this is an new and approved way of doing things?

https://codereview.chromium.org/263143004/diff/10001/public/platform/WebServi...
File public/platform/WebServiceWorkerProviderClient.h (right):

https://codereview.chromium.org/263143004/diff/10001/public/platform/WebServi...
public/platform/WebServiceWorkerProviderClient.h:40: // This class provides
interface for embedders to talk to
Since this is for embedders to talk to Blink, this class should be in public/web
rather than public/platform, shouldn't it? Which would imply an extra a level of
indirection in the impl.

[jsbell, 2014-05-05 19:04:38]

Oh, and a FIXME somewhere referencing crbug.com/351753 would probably be a good
idea.

[kinuko, 2014-05-05 23:26:01]

Thanks, will add a TODO for blob messaging.

https://codereview.chromium.org/263143004/diff/10001/public/platform/WebServi...
File public/platform/WebServiceWorkerProviderClient.h (right):

https://codereview.chromium.org/263143004/diff/10001/public/platform/WebServi...
public/platform/WebServiceWorkerProviderClient.h:40: // This class provides
interface for embedders to talk to
On 2014/05/05 19:03:05, jsbell wrote:
> Since this is for embedders to talk to Blink, this class should be in
public/web
> rather than public/platform, shouldn't it? Which would imply an extra a level
of
> indirection in the impl.

I think we already have multiple interfaces in public/platform that work in this
way, e.g. WebMessagePortChannelClient, WebSocketStreamHandleClient etc, and
public/{platform, web} distinction should be made based on layering rather than
by direction?

[jsbell, 2014-05-06 00:05:21]

https://codereview.chromium.org/263143004/diff/10001/public/platform/WebServi...
File public/platform/WebServiceWorkerProviderClient.h (right):

https://codereview.chromium.org/263143004/diff/10001/public/platform/WebServi...
public/platform/WebServiceWorkerProviderClient.h:40: // This class provides
interface for embedders to talk to
On 2014/05/05 23:26:02, kinuko wrote:
> On 2014/05/05 19:03:05, jsbell wrote:
> > Since this is for embedders to talk to Blink, this class should be in
> public/web
> > rather than public/platform, shouldn't it? Which would imply an extra a
level
> of
> > indirection in the impl.
> 
> I think we already have multiple interfaces in public/platform that work in
this
> way, e.g. WebMessagePortChannelClient, WebSocketStreamHandleClient etc, and
> public/{platform, web} distinction should be made based on layering rather
than
> by direction?

Yeah, direction's not important. But the concrete implementation of 
public/platform/* is usually in Chromium (content/child or content/renderer),
whereas the concrete implementation of public/web/* is usually in WebKit
(Source/web).

(I'd push forward as-is and clean up later, just thought I'd point this out. As
usual, I'm happy to be corrected!)

[kinuko, 2014-05-06 00:53:13]

On 2014/05/06 00:05:21, jsbell wrote:
>
https://codereview.chromium.org/263143004/diff/10001/public/platform/WebServi...
> File public/platform/WebServiceWorkerProviderClient.h (right):
> 
>
https://codereview.chromium.org/263143004/diff/10001/public/platform/WebServi...
> public/platform/WebServiceWorkerProviderClient.h:40: // This class provides
> interface for embedders to talk to
> On 2014/05/05 23:26:02, kinuko wrote:
> > On 2014/05/05 19:03:05, jsbell wrote:
> > > Since this is for embedders to talk to Blink, this class should be in
> > public/web
> > > rather than public/platform, shouldn't it? Which would imply an extra a
> level
> > of
> > > indirection in the impl.
> > 
> > I think we already have multiple interfaces in public/platform that work in
> this
> > way, e.g. WebMessagePortChannelClient, WebSocketStreamHandleClient etc, and
> > public/{platform, web} distinction should be made based on layering rather
> than
> > by direction?
> 
> Yeah, direction's not important. But the concrete implementation of 
> public/platform/* is usually in Chromium (content/child or content/renderer),
> whereas the concrete implementation of public/web/* is usually in WebKit
> (Source/web).
>
> (I'd push forward as-is and clean up later, just thought I'd point this out.
As
> usual, I'm happy to be corrected!)

Ok, I'm pushing this anyway for now.  We have some platform interfaces
implemented by blink (WebKit) like this, hope it's ok or they may have another
cleanup later together.

[kinuko, 2014-05-06 07:47:36]

Jochen: could you do owner review for public/*, Source/web changes?

[kinuko, 2014-05-07 00:55:05]

On 2014/05/06 07:47:36, kinuko wrote:
> Jochen: could you do owner review for public/*, Source/web changes?

Friendly ping.

[jochen (gone - plz use gerrit), 2014-05-07 06:40:14]

this change lgtm.

[kinuko, 2014-05-07 07:17:55]

The CQ bit was checked by kinuko@chromium.org

[commit-bot: I haz the power, 2014-05-07 07:19:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kinuko@chromium.org/263143004/30001

[commit-bot: I haz the power, 2014-05-07 08:26:42]

Change committed as 173495

[loislo, 2014-05-07 13:25:47]

A revert of this CL has been created in
https://codereview.chromium.org/270303004/ by loislo@chromium.org.

The reason for reverting is: Speculative revert in order of fixing crashes in
content_browsertests.

[kinuko, 2014-05-07 15:47:22]

On 2014/05/07 13:25:47, loislo wrote:
> A revert of this CL has been created in
> https://codereview.chromium.org/270303004/ by
https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=loislo@chromium.org.
> 
> The reason for reverting is: Speculative revert in order of fixing crashes in
> content_browsertests.

https://codereview.chromium.org/271543003/ should fix the crash, I'll reland
this one after it lands.

[kinuko, 2014-05-07 17:25:01]

The CQ bit was checked by kinuko@chromium.org

[commit-bot: I haz the power, 2014-05-07 17:26:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kinuko@chromium.org/263143004/30001

[commit-bot: I haz the power, 2014-05-07 17:26:46]

Change committed as 173546

[commit-bot: I haz the power, 2014-05-07 17:27:28]

Failed to apply patch for
Source/modules/serviceworkers/NavigatorServiceWorker.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/modules/serviceworkers/NavigatorServiceWorker.cpp
  Hunk #1 FAILED at 5.
  Hunk #2 FAILED at 21.
  2 out of 2 hunks FAILED -- saving rejects to file
Source/modules/serviceworkers/NavigatorServiceWorker.cpp.rej

Patch:       Source/modules/serviceworkers/NavigatorServiceWorker.cpp
Index: Source/modules/serviceworkers/NavigatorServiceWorker.cpp
diff --git a/Source/modules/serviceworkers/NavigatorServiceWorker.cpp
b/Source/modules/serviceworkers/NavigatorServiceWorker.cpp
index
57ef2a1511c27e672f5c163ede7eaf9076194398..7591f3bd345e56fb0ebee3f2e3d8d2ca4d0f6d61
100644
--- a/Source/modules/serviceworkers/NavigatorServiceWorker.cpp
+++ b/Source/modules/serviceworkers/NavigatorServiceWorker.cpp
@@ -5,6 +5,7 @@
 #include "config.h"
 #include "modules/serviceworkers/NavigatorServiceWorker.h"
 
+#include "core/dom/Document.h"
 #include "core/frame/DOMWindow.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Navigator.h"
@@ -21,12 +22,22 @@ NavigatorServiceWorker::~NavigatorServiceWorker()
 {
 }
 
+NavigatorServiceWorker* NavigatorServiceWorker::from(Document& document)
+{
+    if (!document.frame() || !document.frame()->domWindow())
+        return 0;
+    Navigator& navigator = document.frame()->domWindow()->navigator();
+    return &from(navigator);
+}
+
 NavigatorServiceWorker& NavigatorServiceWorker::from(Navigator& navigator)
 {
     NavigatorServiceWorker* supplement = toNavigatorServiceWorker(navigator);
     if (!supplement) {
         supplement = new NavigatorServiceWorker(navigator);
         provideTo(navigator, supplementName(), adoptPtrWillBeNoop(supplement));
+        // Initialize ServiceWorkerContainer too.
+        supplement->serviceWorker();
     }
     return *supplement;
 }