Bug 201299

Summary:

Move MessageRegistry to NetworkProcess

Product:

WebKit

Reporter:

youenn fablet <youennf>

Component:

Service Workers

Assignee:

youenn fablet <youennf>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, beidson, cdumez, commit-queue, dbates, esprehn+autocc, ews-watchlist, kangil.han, sihui_liu, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

201300, 201333

Bug Blocks:

201400

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch for landing	none

Description youenn fablet 2019-08-29 05:34:23 PDT

This will remove unnecessary hops for service worker messages.

Comment 1 youenn fablet 2019-08-29 05:53:39 PDT

Created attachment 377571 [details]
Patch

Comment 2 youenn fablet 2019-09-02 06:14:01 PDT

Created attachment 377850 [details]
Patch

Comment 3 youenn fablet 2019-09-02 09:02:07 PDT

Created attachment 377853 [details]
Patch

Comment 4 youenn fablet 2019-09-03 04:16:49 PDT

Only downside is that, currently, on network process crash, the registry information is gone and would need to be rebuilt on network process crash recovery.

Comment 5 youenn fablet 2019-09-03 04:18:35 PDT

Comment on attachment 377853 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=377853&action=review

> Source/WebKit/UIProcess/API/APIContextMenuElementInfo.h:35
> +#include <wtf/RetainPtr.h>

here and below: unified build fix apparently.

Comment 6 Alex Christensen 2019-09-03 14:23:28 PDT

Comment on attachment 377853 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=377853&action=review

> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:962
> +void NetworkConnectionToWebProcess::checkRemotePortForActivity(const WebCore::MessagePortIdentifier port, CompletionHandler<void(bool)>&& callback)

It would make things a bit cleaner to make this CompletionHandler<void(HasActivity)>

> Source/WebKit/NetworkProcess/NetworkMessagePortChannelProvider.cpp:68
> +    // Should never be called in the UI process provider.
> +    ASSERT_NOT_REACHED();

Can we remove all these?
Could we at least write code that calls the CompletionHandlers?

Comment 7 youenn fablet 2019-09-03 23:15:27 PDT

(In reply to Alex Christensen from comment #6)
> Comment on attachment 377853 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=377853&action=review
> 
> > Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:962
> > +void NetworkConnectionToWebProcess::checkRemotePortForActivity(const WebCore::MessagePortIdentifier port, CompletionHandler<void(bool)>&& callback)
> 
> It would make things a bit cleaner to make this
> CompletionHandler<void(HasActivity)>

OK

> > Source/WebKit/NetworkProcess/NetworkMessagePortChannelProvider.cpp:68
> > +    // Should never be called in the UI process provider.
> > +    ASSERT_NOT_REACHED();
> 
> Can we remove all these?
> Could we at least write code that calls the CompletionHandlers?

Patch at bug 201400 will fix this.

Comment 8 youenn fablet 2019-09-04 03:46:12 PDT

Created attachment 377969 [details]
Patch for landing

Comment 9 WebKit Commit Bot 2019-09-04 05:46:32 PDT

Comment on attachment 377969 [details]
Patch for landing

Clearing flags on attachment: 377969

Committed r249479: <https://trac.webkit.org/changeset/249479>

Comment 10 WebKit Commit Bot 2019-09-04 05:46:33 PDT

All reviewed patches have been landed.  Closing bug.

Comment 11 Radar WebKit Bug Importer 2019-09-04 05:47:19 PDT

<rdar://problem/55015929>

Comment 12 Alex Christensen 2019-11-22 11:35:35 PST

Comment on attachment 377969 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=377969&action=review

> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:945
> +    auto callback = m_messageBatchDeliveryCompletionHandlers.take(messageBatchIdentifier);
> +    ASSERT(callback);
> +    callback();

This caused https://bugs.webkit.org/show_bug.cgi?id=204460