flypig.co.uk

It looks like good progress from yesterday's changes this morning. Some IMEEnabled items have appeared that I missed or fixed up incorrectly, but also many of the previous errors missing.

195:08.34 In file included from Unified_cpp_mobile_sailfishos0.cpp:128:
195:08.34 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/
          EmbedLitePuppetWidget.cpp:225:13: error: ‘Enabled’ is not a member of
          ‘nsIWidget::IMEState’ {aka ‘mozilla::widget::IMEState’}
195:08.34    IMEState::Enabled enabled = aContext.mIMEState.mEnabled;
195:08.35              ^~~~~~~
195:08.35 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/
          EmbedLitePuppetWidget.cpp:230:7: error: ‘enabled’ was not declared in
          this scope
195:08.35        enabled = IMEEnabled::Disabled;
195:08.35        ^~~~~~~
195:08.35 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/
          EmbedLitePuppetWidget.cpp:230:7: note: suggested alternative: ‘Enable’
195:08.35        enabled = IMEEnabled::Disabled;
195:08.35        ^~~~~~~
195:08.35        Enable
195:08.35 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/
          EmbedLitePuppetWidget.cpp:234:38: error: ‘enabled’ was not declared
          in this scope
195:08.35    mInputContext.mIMEState.mEnabled = enabled;
195:08.35                                       ^~~~~~~
195:08.35 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/
          EmbedLitePuppetWidget.cpp:234:38: note: suggested alternative: ‘Enable’
195:08.35    mInputContext.mIMEState.mEnabled = enabled;
195:08.35                                       ^~~~~~~
195:08.36                                       Enable
195:08.36 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/
          EmbedLitePuppetWidget.cpp: In member function ‘virtual
          mozilla::widget::InputContext mozilla::embedlite::EmbedLitePuppetWidget::
          GetInputContext()’:
195:08.36 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/
          EmbedLitePuppetWidget.cpp:256:35: error: cannot convert ‘mozilla::
          widget::IMEEnabled’ to ‘int32_t’ {aka ‘int’} in initialization
195:08.36      int32_t enabled = IMEEnabled::Disabled;
195:08.36                                    ^~~~~~~~
195:08.36 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/
          EmbedLitePuppetWidget.cpp:260:64: error: ‘Enabled’ in
          ‘nsIWidget::IMEEnabled’ {aka ‘enum class mozilla::widget::IMEEnabled’}
          does not name a type
195:08.36      mInputContext.mIMEState.mEnabled = static_cast(enabled);
195:08.36                                                                 ^~~~~~~
195:08.36 In file included from ${PROJECT}/gecko-dev/widget/nsIWidget.h:27,
195:08.36                  from ${PROJECT}/obj-build-mer-qt-xr/dist/include/
                                mozilla/BasicEvents.h:19,
195:08.36                  from ${PROJECT}/obj-build-mer-qt-xr/dist/include/
                                mozilla/MouseEvents.h:11,
195:08.36                  from ${PROJECT}/obj-build-mer-qt-xr/dist/include/
                                mozilla/dom/Touch.h:12,
195:08.36                  from ${PROJECT}/obj-build-mer-qt-xr/dist/include/
                                mozilla/TouchEvents.h:11,
195:08.36                  from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                embedshared/EmbedLiteViewChildIface.h:6,
195:08.36                  from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                embedshared/EmbedLiteAppChildIface.h:5,
195:08.36                  from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                embedshared/EmbedLiteAppChild.h:11,
195:08.36                  from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                embedthread/EmbedLiteAppThreadChild.h:9,
195:08.36                  from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                EmbedLiteApp.cpp:26,
195:08.36                  from Unified_cpp_mobile_sailfishos0.cpp:2:
195:08.36 ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/widget/IMEData.h:274:3:
          note: ‘Enabled’ declared here
195:08.36    Enabled,
195:08.36    ^~~~~~~
195:08.95 In file included from Unified_cpp_mobile_sailfishos0.cpp:137:
195:08.95 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:
          In member function ‘void mozilla::embedlite::EmbedLiteViewChild::
          InitGeckoWindow(uint32_t, mozilla::dom::BrowsingContext*, bool, bool)’:
195:08.95 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:332:15:
          error: ‘class nsIDocShell’ has no member named ‘SetAffectPrivateSessionLifetime’
195:08.95      docShell->SetAffectPrivateSessionLifetime(true);
195:08.95                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
195:08.96 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:
          In member function ‘virtual mozilla::ipc::IPCResult mozilla::embedlite::
          EmbedLiteViewChild::RecvGoBack()’:
195:08.96 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:560:26:
          error: no matching function for call to ‘nsIWebNavigation::GoBack()’
195:08.96    mWebNavigation->GoBack();
195:08.97                           ^
195:08.97 In file included from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                embedshared/EmbedLiteViewChild.h:14,
195:08.97                  from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                embedprocess/EmbedLiteViewProcessChild.h:9,
195:08.97                  from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                embedprocess/EmbedLiteAppProcessChild.cpp:22,
195:08.97                  from Unified_cpp_mobile_sailfishos0.cpp:56:
195:08.97 ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsIWebNavigation.h:64:14:
          note: candidate: ‘virtual nsresult nsIWebNavigation::GoBack(bool, bool)’
195:08.97    NS_IMETHOD GoBack(bool aRequireUserInteraction, bool aUserActivation) = 0;
195:08.97               ^~~~~~
195:08.97 ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsIWebNavigation.h:64:14:
          note:   candidate expects 2 arguments, 0 provided
195:08.97 In file included from Unified_cpp_mobile_sailfishos0.cpp:137:
195:08.97 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:
          In member function ‘virtual mozilla::ipc::IPCResult mozilla::embedlite::
          EmbedLiteViewChild::RecvGoForward()’:
195:08.97 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:568:29:
          error: no matching function for call to ‘nsIWebNavigation::GoForward()’
195:08.97    mWebNavigation->GoForward();
195:08.97                              ^
195:08.97 In file included from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                embedshared/EmbedLiteViewChild.h:14,
195:08.97                  from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                embedprocess/EmbedLiteViewProcessChild.h:9,
195:08.97                  from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                embedprocess/EmbedLiteAppProcessChild.cpp:22,
195:08.97                  from Unified_cpp_mobile_sailfishos0.cpp:56:
195:08.97 ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsIWebNavigation.h:67:14:
          note: candidate: ‘virtual nsresult nsIWebNavigation::GoForward(bool, bool)’
195:08.97    NS_IMETHOD GoForward(bool aRequireUserInteraction, bool aUserActivation) = 0;
195:08.97               ^~~~~~~~~
195:08.97 ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsIWebNavigation.h:67:14:
          note:   candidate expects 2 arguments, 0 provided
[...]
195:11.46 make[4]: *** [${PROJECT}/gecko-dev/config/rules.mk:676: Unified_cpp_mobile_sailfishos0.o] Error 1

Fixing up the mistakes I made yesterday relating to IMEEnabled was pretty straightforward; just small changes. So let's move on to the next proper error:

195:08.95 In file included from Unified_cpp_mobile_sailfishos0.cpp:137:
195:08.95 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/
          EmbedLiteViewChild.cpp:In member function ‘void mozilla::embedlite::
          EmbedLiteViewChild::InitGeckoWindow(uint32_t, mozilla::dom::
          BrowsingContext*, bool, bool)’:
195:08.95 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/
          EmbedLiteViewChild.cpp:332:15: error: ‘class nsIDocShell’ has no
          member named ‘SetAffectPrivateSessionLifetime’
195:08.95      docShell->SetAffectPrivateSessionLifetime(true);
195:08.95                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

A bit of digging in the logs shows it's related to this change:

$ git log -1 -S affectPrivateSessionLifetime docshell/base/nsIDocShell.idl
commit d9f3a1519c62bee91e5edb82afc038d716cd2b2d
Author: Andreas Farre 
Date:   Mon Jul 5 15:17:55 2021 +0000

    Bug 1701303 - Move counting of private browsing contexts to parent process. r=smaug,johannh
    
    Move the counting of private browsing contexts to the parent
    process. Also change to only consider non-chrome browsing contexts
    when counting private contexts. The latter is possible due to bug
    1528115, because we no longer need to support hidden private windows.
    
    With counting in the parent process we can make sure that when we're
    changing remoteness on a private browsing context the private browsing
    context count never drops to zero. This fixes an issue with Fission,
    where we remoteness changes could transiently have a zero private
    browsing context count, that would be mistaken for the last private
    browsing context going away.
    
    Changing to only count non-chrome browsing contexts makes us only fire
    'last-pb-context-exited' once, and since we count them in the parent
    there is no missing information about contexts that makes us wait for
    a content process about telling us about insertion or removal of
    browsing contexts.
    
    Differential Revision: https://phabricator.services.mozilla.com/D118182

That's a long description, but it essentially means that nsIDocShell no longer has an affectPrivateSessionLifetime attribute. All of the code relating to this has just been stripped out from upstream. We can do the same, but there's a question about whether we, as the parent process, need to be keeping track of private sessions somewhere else instead.

Looking more carefully at the change, the code tracking private sessions now seems to be part of the BrowserContext, so I think we may get this for free. So I've just removed the EmbedLite code that relates to this. There is only one instance anyway. So that turns out to be a lot of explanation for a very small change.

Next we have a series of errors that look similar to this:

195:08.96 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/
          EmbedLiteViewChild.cpp: In member function ‘virtual mozilla::ipc::
          IPCResult mozilla::embedlite::EmbedLiteViewChild::RecvGoBack()’:
195:08.96 ${PROJECT}/gecko-dev/mobile/sailfishos/embedshared/
          EmbedLiteViewChild.cpp:560:26: error: no matching function for call
          to ‘nsIWebNavigation::GoBack()’
195:08.96    mWebNavigation->GoBack();
195:08.97                           ^
195:08.97 In file included from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                embedshared/EmbedLiteViewChild.h:14,
195:08.97                  from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                embedprocess/EmbedLiteViewProcessChild.h:9,
195:08.97                  from ${PROJECT}/gecko-dev/mobile/sailfishos/
                                embedprocess/EmbedLiteAppProcessChild.cpp:22,
195:08.97                  from Unified_cpp_mobile_sailfishos0.cpp:56:
195:08.97 ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsIWebNavigation.h:64:14:
          note: candidate: ‘virtual nsresult nsIWebNavigation::GoBack(bool, bool)’
195:08.97    NS_IMETHOD GoBack(bool aRequireUserInteraction, bool aUserActivation) = 0;
195:08.97               ^~~~~~
195:08.97 ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsIWebNavigation.h:64:14:
          note:   candidate expects 2 arguments, 0 provided

It looks like the signature for several methods defined in docshell/base/nsIWebNavigation.idl have changed. For example, what was this:

  /**
   * Tells the object to navigate to the previous session history item.  When a
   * page is loaded from session history, all content is loaded from the cache
   * (if available) and page state (such as form values and scroll position) is
   * restored.
   *
   * @throw NS_ERROR_UNEXPECTED
   *        Indicates that the call was unexpected at this time, which implies
   *        that canGoBack is false.
   */
  void goBack();

Is now this:

  /**
   * Tells the object to navigate to the previous session history item.  When a
   * page is loaded from session history, all content is loaded from the cache
   * (if available) and page state (such as form values and scroll position) is
   * restored.
   *
   * @param {boolean} aRequireUserInteraction
   *        Tells goBack to skip history items that did not record any user
   *        interaction on their corresponding document while they were active.
   *        This means in case of multiple entries mapping to the same document,
   *        each entry has to have been flagged with user interaction separately.
   *        If no items have user interaction, the function will fall back
   *        to the first session history entry.
   *
   * @param {boolean} aUserActivation
   *        Tells goBack that the call was triggered by a user action (e.g.:
   *        The user clicked the back button).
   *
   * @throw NS_ERROR_UNEXPECTED
   *        Indicates that the call was unexpected at this time, which implies
   *        that canGoBack is false.
   */
  void goBack([optional] in boolean aRequireUserInteraction, [optional] in boolean aUserActivation);

As you can see, that's two new parameters that have been added. You might have thought that those [optional] annotations might mean we could leave them out if we wante, but it seems IDL doesn't work like that (instead it seems to be wrapping them in an "optional" container). Here are the logs for the two commits that combine into this change. You can also see that it's affected a few other methods as well:

$ git log -1 2133bb8e2cf2f
commit 2133bb8e2cf2f68cf1502b57f68e322914cd90b7
Author: Johann Hofmann 
Date:   Tue Jun 9 14:50:14 2020 +0000

    Bug 1515073 - Part 2 - Allow nsIWebNavigation::{goBack,goForward} to skip
    entries without user interaction. r=Gijs,peterv
    
    Depends on D27585
    
    Differential Revision: https://phabricator.services.mozilla.com/D27586

$ git log -1 b97cd2430b0cf
commit b97cd2430b0cf90169060773f5a948bbafbd71a3
Author: Niklas Goegge 
Date:   Wed Apr 28 11:26:49 2021 +0000

    Bug 1708150 - Add user activation flag to reload, goBack and goForward
    r=ckerschb,Gijs,smaug
    
    Differential Revision: https://phabricator.services.mozilla.com/D110245

Fixing all these involves working through the code and adding both parameters in. In most cases I just added the two parameters all the way up to where the methods are exposed. These will then be exposed to QtMozEmbed, which will have to figure out whether to use them or not (my guess is that most of the cases will involve user interaction, but maybe there are exceptions).

It's quite a few small changes, but as usual, nothing too dramatic. I'll talk more about exactly how these were fixed and why they are important tomorrow. But that's it for today.

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