flypig.co.uk

List items

Items from the current list are shown below.

Gecko

13 Sep 2023 : Day 28 #
This morning, following yesterday's disaster, I woke up to check my laptop. And it's good news!

There are errors of course, but these are all new errors. So the changes I made to address the last set of errors (two days ago now!) seem to have done the trick. Already I can feel that today is going to be a better day than yesterday.

I've committed the changes with suitable log messages and can now move on to the next.

There are now — it can't be denied — a lot more errors to tackle. 
516:47.81 In file included from Unified_cpp_mobile_sailfishos0.cpp:128:
516:47.81 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:
          In member function ‘virtual void* mozilla::embedlite::
          EmbedLitePuppetWidget::GetNativeData(uint32_t)’:
516:47.81 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:116:10:
          error: ‘NS_NATIVE_SHAREABLE_WINDOW’ was not declared in this scope
516:47.81      case NS_NATIVE_SHAREABLE_WINDOW: {
516:47.81           ^~~~~~~~~~~~~~~~~~~~~~~~~~
516:47.91 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:116:10:
          note: suggested alternative: ‘NS_NATIVE_TMP_WINDOW’
516:47.91      case NS_NATIVE_SHAREABLE_WINDOW: {
516:47.91           ^~~~~~~~~~~~~~~~~~~~~~~~~~
516:47.91           NS_NATIVE_TMP_WINDOW
516:47.91 In file included from ${PROJECT}/gecko-dev/mobile/sailfishos/utils/EmbedLog.h:10,
516:47.91                  from ${PROJECT}/gecko-dev/mobile/sailfishos/EmbedLiteApp.cpp:6,
516:47.91                  from Unified_cpp_mobile_sailfishos0.cpp:2:
516:47.91 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:
          In member function ‘virtual void mozilla::embedlite::EmbedLitePuppetWidget::SetInputContext
          (const InputContext&, const InputContextAction&)’:
516:47.91 ${PROJECT}/gecko-dev/mobile/sailfishos/utils/EmbedLog.h:19:96: warning:
          format ‘%X’ expects argument of type ‘unsigned int’, but argument 6
          has type ‘mozilla::widget::IMEEnabled’ [-Wformat=]
516:47.91  #define LOGT(FMT, ...) MOZ_LOG(GetEmbedCommonLog("EmbedLiteTrace"),
          mozilla::LogLevel::Debug, ("TRACE::%s:%d " FMT , __PRETTY_FUNCTION__, __LINE__, ##__VA_ARGS__))
516:47.91 ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/Logging.h:221:34:
          note: in definition of macro ‘MOZ_LOG_EXPAND_ARGS’
516:47.91  #define MOZ_LOG_EXPAND_ARGS(...) __VA_ARGS__
516:47.91                                   ^~~~~~~~~~~
516:47.92 ${PROJECT}/gecko-dev/mobile/sailfishos/utils/EmbedLog.h:19:24: note:
          in expansion of macro ‘MOZ_LOG’
516:47.92  #define LOGT(FMT, ...) MOZ_LOG(GetEmbedCommonLog("EmbedLiteTrace"), mozilla::LogLevel::Debug, ("TRACE::%s:%d " FMT , __PRETTY_FUNCTION__, __LINE__, ##__VA_ARGS__))
516:47.92                         ^~~~~~~
516:47.92 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:212:3:
          note: in expansion of macro ‘LOGT’
516:47.92    LOGT("IME: SetInputContext: s=0x%X, 0x%X, action=0x%X, 0x%X",
516:47.92    ^~~~
516:47.92 In file included from Unified_cpp_mobile_sailfishos0.cpp:128:
516:47.92 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:218:48:
          error: ‘DISABLED’ is not a member of ‘nsIWidget::IMEState’ {aka ‘mozilla::widget::IMEState’}
516:47.92    if (aContext.mIMEState.mEnabled != IMEState::DISABLED &&
516:47.92                                                 ^~~~~~~~
516:47.92 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:219:48:
          error: ‘PLUGIN’ is not a member of ‘nsIWidget::IMEState’
          {aka ‘mozilla::widget::IMEState’}
516:47.92        aContext.mIMEState.mEnabled != IMEState::PLUGIN &&
516:47.92                                                 ^~~~~~
516:47.92 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:226:13:
          error: ‘Enabled’ is not a member of ‘nsIWidget::IMEState’ {aka
          ‘mozilla::widget::IMEState’}
516:47.92    IMEState::Enabled enabled = aContext.mIMEState.mEnabled;
516:47.92              ^~~~~~~
516:47.92 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:230:48:
          error: ‘PLUGIN’ is not a member of ‘nsIWidget::IMEState’ {aka
          ‘mozilla::widget::IMEState’}
516:47.92    if (aContext.mIMEState.mEnabled == IMEState::PLUGIN &&
516:47.92                                                 ^~~~~~
[...]
516:50.98 make[4]: *** [${PROJECT}/gecko-dev/config/rules.mk:676: Unified_cpp_mobile_sailfishos0.o] Error 1
The first error is what looks like a missing enum value: NS_NATIVE_SHAREABLE_WINDOW. This doesn't appear anywhere else in the ESR 91 code, nor in the EmbedLite patches, but it does appear in the ESR 78 code. As it turns out it's not an enum at all but a pre-processor definition. It's defined in widget/nsIWidget.h and takes the value 11, like this: 
// Has to match to NPNVnetscapeWindow, and shareable across processes
// HWND on Windows and XID on X11
#define NS_NATIVE_SHAREABLE_WINDOW 11
Let's see what happened to it: 
$ git log -1 -S "NS_NATIVE_SHAREABLE_WINDOW" -- widget/nsIWidget.h
commit a5f0e8f83c652413c089933a9d43223aad7fc6b0
Author: Emilio Cobos Álvarez 
Date:   Mon Apr 19 13:02:33 2021 +0000

    Bug 1706051 - Remove some IPC messages that are unused. r=smaug
    
    Seems they were for plugins, but now they're just dead code.
    
    Differential Revision: https://phabricator.services.mozilla.com/D112539
It seems it got removed along with all of the plugin-related code. That's frustrating, because we have this woven throughout EmbedLite. Maybe the best thing to do is to try to bring it back?

Attempting to revert there is a conflict. A later change switched from using mIsX11Display in widget.cpp to using GdkIsX11Display() instead. Fixing that looks pretty straightforward and safe.

But there's only the one, everything else goes through okay.

That will only have fixed the first error, but now it's time for work so I may as well kick off another build.

Except... that's odd. I almost immediately get this error: 
 3:53.72 error: failed to determine package fingerprint for build script for
           selectors v0.22.0 (${PROJECT}/gecko-dev/servo/components/selectors)
 3:53.72 Caused by:
 3:53.72   failed to determine the most recently modified file in
           ${PROJECT}/gecko-dev/servo/components/selectors
 3:53.72 Caused by:
 3:53.72   failed to determine list of files in
           ${PROJECT}/gecko-dev/servo/components/selectors
 3:53.72 Caused by:
 3:53.72   object not found - no match for id
           (c2790775d65f51d58fd80e4efdb17bd6fe0d4d3a); class=Odb (9); code=NotFound (-3)
 3:53.73 make[4]: *** [${PROJECT}/gecko-dev/config/makefiles/rust.mk:405: force-cargo-library-build] Error 101
Did the revert change one of the servo files? Not according to the diff.

But, after attempting to run again the reason becomes clear: I messed up the --with git_workaround flag when triggering the build. We discussed this flag briefly all the way back on Day 1, but perhaps it's worth exploring what it's for a little further. As I explained then, practically speaking this flag tells the build process to rename the .git folder in the gecko-dev submodule to .git-disabled.

But when we looked at it then I didn't explain why this was necessary. We can see the code that moves the .git folder in the spec file: 
# Move the .git directory out of the way as cargo gets confused and thinks it
# needs to update our submodule.
%if %{with git_workaround}
%__mv %_builddir/.git %_builddir/.git-disabled ||:
%endif
The comments there explain what the issue is, but there's also more in the commit, created by xfade. So I'm not totally clear on the details here, but it seems that, because it's a git repository, cargo searches the local clone for a Cargo.toml file. And there are many: 
$ find . -iname "Cargo.toml" | wc -l
602
This causes it to try to update the git modules, which then causes the error. By moving the .git directory cargo no longer identifies the gecko-dev folder as a git directory, so it skips this step.

Perhaps someone with better knowledge of cargo can fill refine or fix the details here, but at least that's my understanding of it. For OBS builds there's no need to do this because on OBS the tar_git step strips performs this stripping automatically. The long and short of it is that for local builds, the --with git_workaround flag needs to be added to the build command.

Now that I've corrected this the build completes having knocked off a couple of the top errors, so leaving us with this: 
190:28.81 In file included from Unified_cpp_mobile_sailfishos0.cpp:128:
190:28.82 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:218:48:
          error: ‘DISABLED’ is not a member of ‘nsIWidget::IMEState’
          {aka ‘mozilla::widget::IMEState’}
190:28.82    if (aContext.mIMEState.mEnabled != IMEState::DISABLED &&
190:28.82                                                 ^~~~~~~~
190:28.82 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:219:48:
          error: ‘PLUGIN’ is not a member of ‘nsIWidget::IMEState’
          {aka ‘mozilla::widget::IMEState’}
190:28.82        aContext.mIMEState.mEnabled != IMEState::PLUGIN &&
190:28.82                                                 ^~~~~~
190:28.82 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:226:13:
          error: ‘Enabled’ is not a member of ‘nsIWidget::IMEState’
          {aka ‘mozilla::widget::IMEState’}
190:28.82    IMEState::Enabled enabled = aContext.mIMEState.mEnabled;
190:28.82              ^~~~~~~
190:28.82 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:230:48:
          error: ‘PLUGIN’ is not a member of ‘nsIWidget::IMEState’
          {aka ‘mozilla::widget::IMEState’}
190:28.82    if (aContext.mIMEState.mEnabled == IMEState::PLUGIN &&
190:28.82                                                 ^~~~~~
190:28.82 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:232:7:
          error: ‘enabled’ was not declared in this scope
190:28.82        enabled = IMEState::DISABLED;
190:28.82        ^~~~~~~
190:28.82 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:232:7:
          note: suggested alternative: ‘Enable’
190:28.82        enabled = IMEState::DISABLED;
190:28.82        ^~~~~~~
190:28.82        Enable
190:28.82 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:232:27:
          error: ‘DISABLED’ is not a member of ‘nsIWidget::IMEState’
          {aka ‘mozilla::widget::IMEState’}
190:28.82        enabled = IMEState::DISABLED;
190:28.83                            ^~~~~~~~
190:28.83 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:236:38:
          error: ‘enabled’ was not declared in this scope
190:28.83    mInputContext.mIMEState.mEnabled = enabled;
190:28.83                                       ^~~~~~~
190:28.83 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:236:38:
          note: suggested alternative: ‘Enable’
190:28.83    mInputContext.mIMEState.mEnabled = enabled;
190:28.83                                       ^~~~~~~
190:28.83                                       Enable
190:28.83 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:
          In member function ‘virtual mozilla::widget::InputContext mozilla::
          embedlite::EmbedLitePuppetWidget::GetInputContext()’:
190:28.83 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:258:33:
          error: ‘DISABLED’ is not a member of ‘nsIWidget::IMEState’
          {aka ‘mozilla::widget::IMEState’}
190:28.83      int32_t enabled = IMEState::DISABLED;
190:28.83                                  ^~~~~~~~
190:28.83 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:262:62:
          error: ‘Enabled’ in ‘nsIWidget::IMEState’ {aka ‘struct mozilla::widget::IMEState’}
          does not name a type
190:28.83      mInputContext.mIMEState.mEnabled = static_cast(enabled);
190:28.83                                                               ^~~~~~~
190:28.86 In file included from ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/
                                EmbedLiteViewChild.cpp:10,
190:28.86                  from Unified_cpp_mobile_sailfishos0.cpp:137:
190:28.86 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/nsWindow.h: At global scope:
190:28.86 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/nsWindow.h:37:11:
          error: ‘MOZ_MUST_US ’ does not name a type; did you mean ‘MOZ_MUST_USE_TYPE’?
190:28.86    virtual MOZ_MUST_USE nsresult Create(nsIWidget*        aParent,
190:28.86            ^~~~~~~~~~~~
190:28.86            MOZ_MUST_USE_TYPE
190:29.44 In file included from Unified_cpp_mobile_sailfishos0.cpp:137:
190:29.45 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:
          In member function ‘void mozilla::embedlite::EmbedLiteViewChild::InitGeckoWindow
          (uint32_t, mozilla::dom::BrowsingContext*, bool, bool)’:
190:29.45 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:272:154:
          error: no matching function for call to ‘mozilla::dom::BrowsingContext::CreateDetached
          (std::nullptr_t, mozilla::dom::BrowsingContext*&, const nsString&,
          mozilla::dom::BrowsingContext::Type)’
190:29.45    RefPtr browsingContext = BrowsingContext::CreateDetached(nullptr, parentBrowsingContext, EmptyString(), BrowsingContext::Type::Content);
190:29.45                                                                                                                                                           ^
190:29.45 In file included from ${PROJECT}/obj-build-mer-qt-xr/ipc/ipdl/
                                _ipdlheaders/mozilla/dom/DOMTypes.h:28,
190:29.45                  from ${PROJECT}/obj-build-mer-qt-xr/ipc/ipdl/
                                _ipdlheaders/mozilla/embedlite/PEmbedLiteApp.h:22,
190:29.45                  from ${PROJECT}/obj-build-mer-qt-xr/ipc/ipdl/
                                _ipdlheaders/mozilla/embedlite/PEmbedLiteAppParent.h:9,
190:29.45                  from ${PROJECT}/obj-build-mer-qt-xr/dist/include/
                                mozilla/embedlite/EmbedLiteAppParent.h:9,
190:29.45                  from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                embedthread/EmbedLiteAppThreadParent.h:9,
190:29.45                  from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                EmbedLiteApp.cpp:25,
190:29.45                  from Unified_cpp_mobile_sailfishos0.cpp:2:
190:29.45 ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/dom/
          BrowsingContext.h:275:44: note: candidate: ‘static
          already_AddRefed mozilla::dom::
          BrowsingContext::CreateDetached(nsGlobalWindowInner*,
          mozilla::dom::BrowsingContext*, mozilla::dom::BrowsingContextGroup*,
          const nsAString&, mozilla::dom::BrowsingContext::Type, bool)’
190:29.45    static already_AddRefed CreateDetached(
190:29.45                                             ^~~~~~~~~~~~~~
190:29.45 ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/dom/BrowsingContext.h:275:44:
          note:   candidate expects 6 arguments, 4 provided
190:29.45 In file included from Unified_cpp_mobile_sailfishos0.cpp:137:
190:29.45 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:318:10:
          error: ‘class nsIDOMWindowUtils’ has no member named ‘GetOuterWindowID’
190:29.45    utils->GetOuterWindowID(&mOuterId);
190:29.45           ^~~~~~~~~~~~~~~~
190:29.45 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:333:15:
          error: ‘class nsIDocShell’ has no member named ‘SetAffectPrivateSessionLifetime’
190:29.45      docShell->SetAffectPrivateSessionLifetime(true);
190:29.45                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[...]
190:31.98 make[4]: *** [${PROJECT}/gecko-dev/config/rules.mk:676: Unified_cpp_mobile_sailfishos0.o] Error 1
Lots of juicy errors to address there. Several of them seem to be related to changes made for Bugzilla bug 1683226, so let's address those first. 
190:28.81 In file included from Unified_cpp_mobile_sailfishos0.cpp:128:
190:28.82 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:218:48:
          error: ‘DISABLED’ is not a member of ‘nsIWidget::IMEState’
          {aka ‘mozilla::widget::IMEState’}
190:28.82    if (aContext.mIMEState.mEnabled != IMEState::DISABLED &&
190:28.82                                                 ^~~~~~~~
There are several bugs like the above related to IMEState. In turns out this is due to an upstream refactoring which changed the enum into an enum class: 
$ git log -1 -S "enum Enabled" -- widget/IMEData.h
commit d27602eee6e0ca33bc17a7676d0430d572c36359
Author: Masayuki Nakano 
Date:   Mon Dec 21 05:52:03 2020 +0000

    Bug 1683226 - part 1: Make `IMEState::Enabled` an enum class r=m_kato,geckoview-reviewers
    
    Before deleting `IMEState::Enabled::PLUGIN`, let's make it an enum class
    for making the change safer.  Almost all of this change is done by
    "replace" of VSCode.
    
    Differential Revision: https://phabricator.services.mozilla.com/D100100
A few hours later the same author of this bug then removed the Plugin enum value completely: 
$ git log -1 -S "Plugin," -- widget/IMEData.h
commit 9e229babfad5aead7ef6c445f663af607b469fbb
Author: Masayuki Nakano 
Date:   Mon Dec 21 08:26:24 2020 +0000

    Bug 1683226 - part 11: Get rid of `IMEEnabled::Plugin` r=m_kato
    
    Differential Revision: https://phabricator.services.mozilla.com/D100123
This enum gets used in the EmbedLite code, so I've been through and changed it to use the new enum class. It looks like all the plugin code has been removed from upstream, so I just removed the references to this and related code as well. That may come back to haunt us. Hopefully not!

That deals with the first ten or so bugs. Then we have this one: 
190:29.44 In file included from Unified_cpp_mobile_sailfishos0.cpp:137:
190:29.45 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:
          In member function ‘void mozilla::embedlite::EmbedLiteViewChild::
          InitGeckoWindow(uint32_t, mozilla::dom::BrowsingContext*, bool, bool)’:
190:29.45 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:272:154:
          error: no matching function for call to ‘mozilla::dom::BrowsingContext::
          CreateDetached(std::nullptr_t, mozilla::dom::BrowsingContext*&, const
          nsString&, mozilla::dom::BrowsingContext::Type)’
190:29.45    RefPtr browsingContext = BrowsingContext::CreateDetached(nullptr, parentBrowsingContext, EmptyString(), BrowsingContext::Type::Content);
190:29.45                                                                                                                                                           ^
Checking the log we can see what caused this too: 
$ git log -1 cafcceeb348f0
commit cafcceeb348f0c0e1533d8bfcab6b0a2fb7a948b
Author: Nika Layzell 
Date:   Mon Jul 6 20:10:43 2020 +0000

    Bug 1599579 - Part 1: Add the ability to specify a specific BrowsingContextGroup
    during process switch, r=kmag
    
    Differential Revision: https://phabricator.services.mozilla.com/D80254
A new BrowsingContextGroup parameter has been added to the BrowsingContext::CreateDetached() method, which means that the call to it in the EmbedLite code no longer matches the method signature the upstream code expects. Looking at the changes to the body of the method in the diff, it's clear that we can replicate the same functionality that we have now by simply passing in a nullptr for the missing argument. So this one is easily fixed.

Finally we have this: 
190:29.45 In file included from Unified_cpp_mobile_sailfishos0.cpp:137:
190:29.45 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:318:10:
          error: ‘class nsIDOMWindowUtils’ has no member named ‘GetOuterWindowID’
190:29.45    utils->GetOuterWindowID(&mOuterId);
190:29.45           ^~~~~~~~~~~~~~~~
I had to dig a bit to find out where the change happened for this, because it's hidden in an idl file. Recall these were the interface definition language files that allow interoperability between C++ and JavaScript, used to generate C++ header files. Here's the upstream change that's causing the problems: 
$ git log -1 0c976d908ae42e9ba9c88
commit 0c976d908ae42e9ba9c8899253a1313469256308
Author: Kris Maglione 
Date:   Mon Aug 17 20:22:12 2020 +0000

    Bug 1651519: Part 2 - Also remove nsIDOMWindowUtils::outerWindowID.
    r=nika,geckoview-reviewers,agi
    
    Differential Revision: https://phabricator.services.mozilla.com/D82957
The outerWindowID parameter has been removed from nsIDOMWindowUtils. Luckily what looks like the same value is also exposed by the nsIDocShell interface and this interface is easy to access from all the places we're currently using nsIDOMWindowUtils. So to fix this I've just accessed this different interface and used the getter that's available there. This means there's minimal changes needed to the EmbedLite code.

It's hot and late here, so I'll kick the build off and see if these changes gained us any progress in the morning.

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

Comments

Uncover Disqus comments

Actions

Login Email Share

Archives

View