flypig.co.uk

Finally! EmbedLiteCompositorBridgeParent is now compiling without error. Wahey!

This is a nice step forwards. But it brings new errors in the EmbedLite code. Lots of them. These errors don't appear to be related to the removal of GLScreenBuffer, which caused all of the problems yesterday and over the last few days, which is encouraging. But deeper investigation might prove otherwise.

Here are the some of the new errors. I've not included them in all their gory — and lengthy ‐ detail because there are so many (there were eight times as many lines of error output that I've not included).

This gives a pretty good flavour though, and also contains the first few errors that I'll be wanting to try to fix.

228:28.17 mobile/sailfishos
229:03.66 In file included from $PROJECT/gecko-dev/mobile/sailfishos/EmbedLiteApp.cpp:34,
229:03.66                  from Unified_cpp_mobile_sailfishos0.cpp:2:
229:03.66 $PROJECT/gecko-dev/mobile/sailfishos/embedprocess/EmbedLiteAppProcessParent.h:34:8:
          error: ‘void mozilla::embedlite::EmbedLiteAppProcessParent::
          OnChannelConnected(int32_t)’ marked ‘override’, but does not override
229:03.66    void OnChannelConnected(int32_t pid) override;
229:03.66         ^~~~~~~~~~~~~~~~~~
229:05.23 In file included from Unified_cpp_mobile_sailfishos0.cpp:56:
229:05.23 $PROJECT/gecko-dev/mobile/sailfishos/embedprocess/EmbedLiteAppProcessChild.cpp:
          In member function ‘bool mozilla::embedlite::EmbedLiteAppProcessChild::
          Init(MessageLoop*, base::ProcessId, mozilla::embedlite::
          PEmbedLiteAppChild::UniquePtr&>::type,
          base::ProcessId&, MessageLoop*&)’
229:05.24    if (!Open(std::move(aChannel), aParentPid, aIOLoop)) {
229:05.24                                                      ^
229:05.24 In file included from $PROJECT/obj-build-mer-qt-xr/ipc/ipdl/_ipdlheaders/
                                mozilla/embedlite/PEmbedLiteAppParent.h:16,
229:05.24                  from $PROJECT/obj-build-mer-qt-xr/dist/include/mozilla/
                                embedlite/EmbedLiteAppParent.h:9,
229:05.24                  from $PROJECT/gecko-dev/mobile/sailfishos/embedthread/
                                EmbedLiteAppThreadParent.h:9,
229:05.24                  from $PROJECT/gecko-dev/mobile/sailfishos/
                                EmbedLiteApp.cpp:25,
229:05.24                  from Unified_cpp_mobile_sailfishos0.cpp:2:
229:05.24 $PROJECT/obj-build-mer-qt-xr/dist/include/mozilla/ipc/ProtocolUtils.h:447:8:
          note: candidate: ‘bool mozilla::ipc::IToplevelProtocol::Open(
          mozilla::ipc::ScopedPort, base::ProcessId)’
229:05.24    bool Open(ScopedPort aPort, base::ProcessId aOtherPid);
229:05.24         ^~~~
229:05.24 $PROJECT/obj-build-mer-qt-xr/dist/include/mozilla/ipc/ProtocolUtils.h:447:8:
          note:   candidate expects 2 arguments, 3 provided
229:05.24 $PROJECT/obj-build-mer-qt-xr/dist/include/mozilla/ipc/ProtocolUtils.h:449:8:
          note: candidate: ‘bool mozilla::ipc::IToplevelProtocol::Open(
          mozilla::ipc::MessageChannel*, nsISerialEventTarget*, mozilla::ipc::Side)’
229:05.24    bool Open(MessageChannel* aChannel, nsISerialEventTarget* aEventTarget,
229:05.24         ^~~~
229:05.24 $PROJECT/obj-build-mer-qt-xr/dist/include/mozilla/ipc/ProtocolUtils.h:449:8:
          note:   no known conversion for argument 1 from ‘std::remove_reference&>::type’ {aka ‘mozilla::UniquePtr’}
          to ‘mozilla::ipc::MessageChannel*’
229:05.35 In file included from $PROJECT/gecko-dev/mobile/sailfishos/embedprocess/
          EmbedLiteAppProcessParent.cpp:38,
229:05.35                  from Unified_cpp_mobile_sailfishos0.cpp:65:
229:05.35 $PROJECT/gecko-dev/mobile/sailfishos/embedprocess/
          EmbedLiteCompositorProcessParent.h: At global scope:
229:05.35 $PROJECT/gecko-dev/mobile/sailfishos/embedprocess/
          EmbedLiteCompositorProcessParent.h:21:102: error: ISO C++ forbids declaration
          of ‘NS_INLINE_DECL_THREADSAFE_REFCOUNTING_WITH_MAIN_THREAD_DESTRUCTION’
          with no type [-fpermissive]
229:05.35    NS_INLINE_DECL_THREADSAFE_REFCOUNTING_WITH_MAIN_THREAD_DESTRUCTION(EmbedLiteCompositorProcessParent)
229:05.35                                                                                                       ^
229:05.35 $PROJECT/gecko-dev/mobile/sailfishos/embedprocess/EmbedLiteCompositorProcessParent.h:21:102:
          error: expected ‘;’ at end of member declaration
229:05.35    NS_INLINE_DECL_THREADSAFE_REFCOUNTING_WITH_MAIN_THREAD_DESTRUCTION(EmbedLiteCompositorProcessParent)
229:05.35                                                                                                       ^
229:05.35                                                                                                        ;
229:05.35 In file included from $PROJECT/gecko-dev/mobile/sailfishos/embedprocess/
          EmbedLiteAppProcessParent.cpp:38,
229:05.35                  from Unified_cpp_mobile_sailfishos0.cpp:65:
229:05.35 $PROJECT/gecko-dev/mobile/sailfishos/embedprocess/
          EmbedLiteCompositorProcessParent.h:27:35: error: ‘virtual
          mozilla::ipc::IPCResult mozilla::embedlite::
          EmbedLiteCompositorProcessParent::RecvGetFrameUniformity(mozilla::
          layers::FrameUniformityData*)’ marked ‘override’, but does not override
229:05.36    virtual mozilla::ipc::IPCResult RecvGetFrameUniformity(FrameUniformityData* aOutData) override { return IPC_OK(); }
229:05.36                                    ^~~~~~~~~~~~~~~~~~~~~~
229:05.36 $PROJECT/gecko-dev/mobile/sailfishos/embedprocess/
          EmbedLiteCompositorProcessParent.h:61:16: error: ‘virtual void mozilla::
          embedlite::EmbedLiteCompositorProcessParent::SetConfirmedTargetAPZC(
          const LayersId&, const uint64_t&, const nsTArray&)’ marked ‘override’, but does not override
229:05.36    virtual void SetConfirmedTargetAPZC(const LayersId& aLayersId,
229:05.36                 ^~~~~~~~~~~~~~~~~~~~~~
[...]
229:08.93 make[4]: *** [$PROJECT/gecko-dev/config/rules.mk:676: Unified_cpp_mobile_sailfishos0.o] Error 1

Let's consider the first error in this long list:

229:03.66 $PROJECT/gecko-dev/mobile/sailfishos/embedprocess/EmbedLiteAppProcessParent.h:34:8:
          error: ‘void mozilla::embedlite::EmbedLiteAppProcessParent::
          OnChannelConnected(int32_t)’ marked ‘override’, but does not override
229:03.66    void OnChannelConnected(int32_t pid) override;
229:03.66         ^~~~~~~~~~~~~~~~~~

In ESR 78 the OnChannelConnected() method is introduced into EmbedLiteAppProcessParent through inheritance from IToplevelProtocol, which can be found in the ProtoclUtils.h header file via PEmbedLiteAppParent and PEmbedLiteAppParent. It's a circuitous route.

The method was removed by Bugzilla Bug 1713148 "Part 4: Remove ProcessLink" and diff D116665. The log summary reads like this:

commit 9ae1129462b65b7b75246fb61cf699c73fd8fed0
Author: Nika Layzell 
Date:   Tue Jun 22 18:17:21 2021 +0000

    Bug 1706374 - Part 10: Remove unnecessary IToplevelProtocol::OnChannelConnected, r=handyman,jgilbert
    
    Differential Revision: https://phabricator.services.mozilla.com/D116665

The diff is pretty unremarkable. But the related bug report is epic in its proportions. The broader aim of these changes seems to have been to introduce efficient top-level protocols that don't require spawning so many processes:
 
Simpler is good. But I don't know really know what this means in practice for the code, other than that it affects the IPC mechanism in important ways. More specifically the changes seem to be related to Mojo, possibly replacing its use in some places with a protocol that's internal to Gecko; but I could be misreading that.

Anyway I guess the question that's of peak interest to me right now is: how much has this change affected the interfaces that EmbedLite uses?

I've spent the rest of the evening reading through the patches associated with 1713148. It's clear that the main purpose of the patch is to switch from spawning processes to using ports when performing IPC (although I don't understand what that really means in practice) and that some of the functionality provided by the former no longer applies to the latter.

What seems odd to me is that I'm not seeing how the dropped functionality was reproduced using other means. That's important for this analysis: right now I see a method in the EmbedLite code that's executed by the OnChannelConnected() override. If that method no longer exists, the contents of the override should be moved somewhere else. I'm not currently seeing where that is.

I've done more reading than coding today, which isn't super-fulfilling. But hopefully it will bear fruit later... preferably tomorrow. This might also be a good topic to raise in one of the Mozilla matrix channels.

As always, for other posts, check out my full Gecko Dev Diary.