flypig.co.uk

List items

Items from the current list are shown below.

Gecko

24 Sep 2023 : Day 39 #
Yesterday I fixed a collection of errors by disabling the WebRTC code. Hopefully it should be possible to re-enable it later as it's important functionality.

Arriving at my laptop this morning and scanning the build output, I can't see any source code errors. But there is a build chain error:
257:28.82 toolkit/xre
258:07.86 make[4]: *** No rule to make target 'moc_nsNativeAppSupportQt.cpp',
          needed by 'moc_nsNativeAppSupportQt.o'.  Stop.
258:07.86 make[3]: *** [${PROJECT}/gecko-dev/config/recurse.mk:72: toolkit/xre/target-objects] Error 2
You may recall we had something similar happened on Day 15 when it was a case of there being no rule for moc_message_pump_qt.cc.

Back then we fixed this issue by applying patch 0007 "Disable MOC code generation for message_pump_qt", which made this crucial change:
index c76bb7ebbd6f..d950dabc94a8 100644
--- a/ipc/chromium/moz.build
+++ b/ipc/chromium/moz.build
@@ -111,7 +111,6 @@ if os_bsd or os_linux:
         ]
     if CONFIG['MOZ_ENABLE_QT']:
         SOURCES += [
-            '!moc_message_pump_qt.cc',
             'src/base/message_pump_qt.cc',
         ]
 
So we may be looking to do something similar for moc_nsNativeAppSupportQt.cpp. There is already some reference to this in the build system, changes that were introduced with patch 0002 "Bring back Qt layer", in toolkit/xrc/moz.build:
elif CONFIG['MOZ_WIDGET_TOOLKIT'] == 'qt':
    EXPORTS += ['nsQAppInstance.h']
    SOURCES += [
        '!moc_nsNativeAppSupportQt.cpp',
        'nsNativeAppSupportQt.cpp',
        'nsQAppInstance.cpp',
    ]
I've removed the '!moc_nsNativeAppSupportQt.cpp' line. It's a bit odd because this was added by the patch. Suspicious. But doing a full rebuild takes us to a new area, so the change had some (presumably beneficial) effect.

The next error:
209:24.35 ${PROJECT}/gecko-dev/widget/qt/MediaKeysEventSourceFactory.cpp:9:15:
          error: ‘MediaControlKeysEventSource’ in namespace ‘mozilla::dom’ does
          not name a type
209:24.35  mozilla::dom::MediaControlKeysEventSource* CreateMediaControlKeysEventSource() {
209:24.35                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
At some point between ESR 78 and ESR 91 the classes were renamed from MediaControlKeysEvent* to MediaControlKey* (note the singular Key rather than plural Keys).
$ git log -1 934302cd0da173cba17e6738a82e3cd18eed6865
commit 934302cd0da173cba17e6738a82e3cd18eed6865
Author: alwu 
Date:   Tue Jun 9 02:59:57 2020 +0000

    Bug 1640998 - part9 : use `MediaControlKey` to replace `MediaControlKeysEvent` r=chunmin,agi,geckoview-reviewers
    
    This patch will
    - remove `MediaControlKeysEvent` and use `MediaControlKey` to replace it
    - rename names for all `MediaControlKey` related methods, functions, classes and descriptions
    
    The advantage of doing so are
    - remove the duplicated type so that we only need to maintain `MediaControlKey`
    
    Differential Revision: https://phabricator.services.mozilla.com/D78140
So to fix this I've just made the following change:
-mozilla::dom::MediaControlKeysEventSource* CreateMediaControlKeysEventSource() {
+mozilla::dom::MediaControlKeySource* CreateMediaControlKeySource() {
Next up we have the following errors in ProcInfo:
${PROJECT}/gecko-dev/widget/qt/ProcInfo.cpp: In member function ‘nsresult
  mozilla::StatReader::UseToken(int32_t, const nsAString&, mozilla::ProcInfo&)’:
${PROJECT}/gecko-dev/widget/qt/ProcInfo.cpp:99:15: error:
  ‘struct mozilla::ProcInfo’ has no member named ‘virtualMemorySize’
         aInfo.virtualMemorySize = Get64Value(aToken, &rv);
               ^~~~~~~~~~~~~~~~~
${PROJECT}/gecko-dev/widget/qt/ProcInfo.cpp: In lambda function:
${PROJECT}/gecko-dev/widget/qt/ProcInfo.cpp:243:23: error: no match for
  ‘operator=’ (operand types are ‘nsCString’ {aka ‘nsTString’} and ‘const
  nsTString’)
         info.origin = originCopy;
                       ^~~~~~~~~~
Based on the upstream Bugzilla bug 1659828 the virtualMemorySize attribute has been completely removed from ProcInfo, justified like this:
 
The current definition of virtualMemorySize in windows/ProcInfo.cpp is set to PagefileUsage, which is entirely unrelated to virtual memory size. We should entirely get rid of this statistics on all platforms because we have no scenario in which it can be useful in the first place. If we ever need it, we'll reimplement it correctly. The objective of this bug is to:
  1. change the definitions of structs in ChromeUtils.webidl to remove field virtualMemorySize entirely;
  2. during a rebuild, this will cause a number of compilation issues, fix these by removing all instances of VirtualMemorySize at the site of errors;
  3. fix the test browser_test_procinfo.js to remove virtualMemorySize.

We should follow along with this. The only reason this hasn't already been removed from the qt/ProcInfo.cpp file is presumably just because Mozilla aren't using the Qt version of this code (in either their products or their continuous integration pipelines). The portion to remove is made clear in diff D89013: just remove the case where this is used entirely.

There's this second error to address in ProcInfo.cpp as well:
${PROJECT}/gecko-dev/widget/qt/ProcInfo.cpp: In lambda function:
${PROJECT}/gecko-dev/widget/qt/ProcInfo.cpp:238:23: error: no match for ‘operator=’ (operand types are ‘nsCString’ {aka ‘nsTString’} and ‘const nsTString’)
         info.origin = originCopy;
                       ^~~~~~~~~~
Thankfully the reason looks pretty clear here as well. The origin attribute of ProcInfo has changed its string type. From this:
  // Origin, if any
  nsString origin;
To this:
  // Origin, if any
  nsCString origin;
I tried updating the type of originCopy to match:
diff --git a/widget/qt/ProcInfo.cpp b/widget/qt/ProcInfo.cpp
index 1538b561b4c2..58943c782cc6 100644
--- a/widget/qt/ProcInfo.cpp
+++ b/widget/qt/ProcInfo.cpp
@@ -226,3 +221,3 @@ RefPtr GetProcInfo(base::ProcessId pid, int32_t childId,
   // Ensure that the string is still alive when the runnable is called.
-  nsString originCopy(origin);
+  nsCString originCopy(origin);
   RefPtr r = NS_NewRunnableFunction(
But this then caused other errors. In looking into this deeper it became clear that the GetProcInfo() method that this forms part of has changed quite considerably. Even the signature has changed, from this:
RefPtr GetProcInfo(base::ProcessId pid, int32_t childId,
                                    const ProcType& type, const nsAString& origin)
To this:
RefPtr GetProcInfo(nsTArray&& aRequests)
There's a new implementation too, which can be found in the ProcInfo_linux.cpp file. This therefore needs a more careful fix. To this end I've copied over and fixed up this entirely new implementation to the Qt version of this class.

Now we have a bit of an odd error:
${PROJECT}/gecko-dev/widget/qt/nsAppShell.cpp:8:10: fatal error: nsAppShell.h:
  No such file or directory
 #include "nsAppShell.h"
          ^~~~~~~~~~~~~~
compilation terminated.
It seems the widget/qt/nsAppShell.h file has been completely removed. The other versions are all there and at least the Gtk version hasn't changed since the ESR 78. So I've copied the file back over from ESR 78. Maybe this was an error I made while applying an earlier patch. Then we have some printing-related errors. Although Sailfish Browser doesn't support printing to arbitrary printers we do use the print functionality to allow pages to be saved out in PDF format. So when fixing up these print errors it's important to try to get things right.

Here's the first error:
In file included from ${PROJECT}/gecko-dev/widget/qt/nsDeviceContextSpecQt.cpp:17:
${PROJECT}/gecko-dev/widget/qt/nsDeviceContextSpecQt.h:12:10: fatal error:
  nsIPrinterEnumerator.h: No such file or directory
 #include "nsIPrinterEnumerator.h"
          ^~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
In practice although we use the print functionality, I don't think we're using the printer enumeration code. So to fix this I just removed the nsIPrinterEnumerator class entirely. Then we have a bunch of errors pointing out that the print settings class has changed its interface.
In file included from ${PROJECT}/gecko-dev/widget/qt/nsDeviceContextSpecQt.cpp:24:
${PROJECT}/gecko-dev/widget/qt/nsPrintSettingsQt.h:24:16: error: ‘virtual
  nsresult nsPrintSettingsQt::GetPrintRange(int16_t*)’ marked ‘override’, but
  does not override
     NS_IMETHOD GetPrintRange(int16_t* aPrintRange) override;
                ^~~~~~~~~~~~~
Luckily there's a good base implementation in widget/nsPrintSettingsImpl.cpp to follow, so updating the Qt version is pretty straightforward. It may need some tweaking later and it takes a while to work my way through the details, but sure enough, this does the trick and gets the error resolved.

Next up we have a bunch of errors from the Qt version of nsLookAndFeel. It seems this interface has changed quite a bit too.
In file included from ${PROJECT}/gecko-dev/widget/qt/nsLookAndFeel.cpp:22:
${PROJECT}/gecko-dev/widget/qt/nsLookAndFeel.h:31:18: error: ‘virtual bool
  nsLookAndFeel::GetFontImpl(mozilla::LookAndFeel::FontID, nsString&,
  gfxFontStyle&)’ marked ‘override’, but does not override
     virtual bool GetFontImpl(FontID aID, nsString& aName, gfxFontStyle& aStyle) override;
                  ^~~~~~~~~~~
${PROJECT}/gecko-dev/widget/qt/nsLookAndFeel.h:32:22: error: ‘virtual nsresult
  nsLookAndFeel::GetIntImpl(mozilla::LookAndFeel::IntID, int32_t&)’ marked
  ‘override’, but does not override
     virtual nsresult GetIntImpl(IntID aID, int32_t &aResult) override;
                      ^~~~~~~~~~
${PROJECT}/gecko-dev/widget/qt/nsLookAndFeel.h:33:22: error: ‘virtual nsresult nsLookAndFeel::GetFloatImpl(mozilla::LookAndFeel::FloatID, float&)’ marked
  ‘override’, but does not override
     virtual nsresult GetFloatImpl(FloatID aID, float &aResult) override;
                      ^~~~~~~~~~~~
${PROJECT}/gecko-dev/widget/qt/nsLookAndFeel.h:39:22: error: ‘virtual nsresult
  nsLookAndFeel::NativeGetColor(mozilla::LookAndFeel::ColorID, nscolor&)’ marked
  ‘override’, but does not override
     virtual nsresult NativeGetColor(ColorID aID, nscolor &aColor) override;
                      ^~~~~~~~~~~~~~
${PROJECT}/gecko-dev/widget/qt/nsLookAndFeel.cpp: In member function ‘virtual
  nsresult nsLookAndFeel::GetIntImpl(mozilla::LookAndFeel::IntID, int32_t&)’:
${PROJECT}/gecko-dev/widget/qt/nsLookAndFeel.cpp:369:36: error: ‘GetIntImpl’ is
  not a member of ‘nsXPLookAndFeel’
     nsresult rv = nsXPLookAndFeel::GetIntImpl(aID, aResult);
                                    ^~~~~~~~~~
${PROJECT}/gecko-dev/widget/qt/nsLookAndFeel.cpp:371:38: error:
  ‘eIntID_SystemUsesDarkTheme’ was not declared in this scope
     if (NS_SUCCEEDED(rv) && ((aID != eIntID_SystemUsesDarkTheme) || (aResult != 2)))
                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~
${PROJECT}/gecko-dev/widget/qt/nsLookAndFeel.cpp:377:14: error:
  ‘eIntID_CaretBlinkTime’ was not declared in this scope
         case eIntID_CaretBlinkTime:
              ^~~~~~~~~~~~~~~~~~~~~
${PROJECT}/gecko-dev/widget/qt/nsLookAndFeel.cpp:381:14: error:
  ‘eIntID_CaretWidth’ was not declared in this scope
         case eIntID_CaretWidth:
              ^~~~~~~~~~~~~~~~~
There are quite a range of errors here. Some are due to methods being renamed in the parent nsXPLookAndFeel class. Many of them are due to enums being switched to use enum classes. One of them is due to the string literal macros having been changed. But most of them are pretty easy to fix.

But after fixing all of these there are yet more errors to tackle. This does feel rather like a never ending story.
In file included from ${PROJECT}/gecko-dev/widget/qt/nsNativeThemeQt.cpp:5:
${PROJECT}/gecko-dev/widget/qt/nsNativeThemeQt.h:16:14: error: ‘virtual nsresult
  nsNativeThemeQt::DrawWidgetBackground(gfxContext*, nsIFrame*,
  nsITheme::StyleAppearance, const nsRect&, const nsRect&)’ marked ‘override’,
  but does not override
   NS_IMETHOD DrawWidgetBackground(gfxContext* aContext, nsIFrame* aFrame,
              ^~~~~~~~~~~~~~~~~~~~
${PROJECT}/gecko-dev/widget/qt/nsNativeThemeQt.cpp: In function ‘void
  PaintCheckboxControl(nsIFrame*, mozilla::gfx::DrawTarget*, const nsRect&,
  const mozilla::EventStates&)’:
${PROJECT}/gecko-dev/widget/qt/nsNativeThemeQt.cpp:49:24: error: ‘NSRectToRect’
  was not declared in this scope
   Rect shadowGfxRect = NSRectToRect(paddingRect, twipsPerPixel);
                        ^~~~~~~~~~~~
${PROJECT}/gecko-dev/widget/qt/nsNativeThemeQt.cpp: In function ‘void
  PaintCheckMark(nsIFrame*, mozilla::gfx::DrawTarget*, const nsRect&)’:
${PROJECT}/gecko-dev/widget/qt/nsNativeThemeQt.cpp:91:19: error: ‘NSPointToPoint’
  was not declared in this scope
   builder->MoveTo(NSPointToPoint(p, appUnitsPerDevPixel));
                   ^~~~~~~~~~~~~~
I've already ploughed through quite a few errors and I think I've reached my limit, so these are going to have to wait until tomorrow. But it does feel like I've been able to make some progress today and I'm hoping that we really are coming close to the end of the build-blocking errors now. As always, there will be more tomorrow!

And of course, if you want to read my other posts related to this, you can find them in my Gecko Dev Diary.

Comments

Uncover Disqus comments