Bug 201299 - Move MessageRegistry to NetworkProcess
Summary: Move MessageRegistry to NetworkProcess
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Service Workers (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on: 201300 201333
Blocks: 201400
  Show dependency treegraph
 
Reported: 2019-08-29 05:34 PDT by youenn fablet
Modified: 2019-12-18 13:24 PST (History)
10 users (show)

See Also:


Attachments
Patch (131.92 KB, patch)
2019-08-29 05:53 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (67.71 KB, patch)
2019-09-02 06:14 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (68.62 KB, patch)
2019-09-02 09:02 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (70.49 KB, patch)
2019-09-04 03:46 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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