List items
Items from the current list are shown below.
Blog
22 Feb 2024 : Day 164 #
Working on the WebView implementation, yesterday we reached the point where the WebView component no longer crashed the app hosting it. We did this by ensuring the correct layer manager was used for rendering.
But now we're left with a bunch of errors. The ones that need fixing immediately are the following:
It seems a bit odd that a text selector component should be requiring an entire separate phone library in order to work. Let's take a look at why.
The ofono code comes at the end of the file. There are two OfonoNetworkRegistration components called cellular1Status and cellular2Status. These represent the state of the two SIM card slots in the device. You might ask why there are only two; can't you have more than two SIM card slots? Well, yes, but I guess this is a problem for future developers to deal with.
These two components feed into the following Boolean value at the top of the code:
You might wonder how it knows it's a phone number at all. The answer to this question isn't in the sailfish-components-webview code. The answer is in embedlite-components, in the SelectionPrototype.js file where we find this code:
But that's a bit of a diversion. We only care about this new libqofono. Why is this newer version needed and why don't I have it on my system? Let's find out when and why it was changed. $ git blame import/controls/TextSelectionController.qml -L 14,14 16ef5cdf4 (Pekka Vuorela 2023-01-05 12:09:27 +0200 14) import QOfono 0.2 $ git log -1 16ef5cdf4 commit 16ef5cdf44c2eafd7d93e17a41927ef5da700c2b Author: Pekka Vuorela <pekka.vuorela@jolla.com> Date: Thu Jan 5 12:09:27 2023 +0200 [components-webview] Migrate to new qofono import. JB#59690 Also dependency was missing. The actual change here was pretty small.
None of this seems essential for ESR 91. My guess is that this change has gone into the development code but hasn't yet made it into a release. So I'm going to hack around it for now (being careful not to commit my hacked changes into the repository).
I've already amended the version number in the spec file, so to get things to work I should just have to reverse this change:
[...]
It's the evening and time to put the harbour-webview example through the debugger.
At the time I struggled with how to rearrange the code so that it compiled. I made changes that I couldn't test. And while I did get it to compile, these changes are now coming back to haunt me. Now I need to actually fix this rendering pipeline properly.
There's a bit of me that is glad I'm finally having to do this. I really want to know how it's actually supposed to work.
Clearly the first task will be to figure out why the mSwapChain member of GLContext is never being set. With any luck this will be at the easier end of the difficulty spectrum.
I'm going to try to find where mSwapChain is being — or should be being — set. To do that I'll need to find out where the context is coming from. The context is being passed by CompositorOGL so that would seem to be a good place to start.
Looking through the CompositoryOGL.cpp file we can see that the mGLContext member is being initialised from a value passed in to CompositorOGL::Initialize(). The debugger can help us work back from there.
But my head is full and this feels like a good place to leave things. Inside this method might be the right place to initialise mSwapChain, but it's definitely not happening here.
Tomorrow I'll do a sweep of the other code to check whether any attempt is being made to initialise it somewhere else. If not I'll add in some initialisation code to see what happens.
If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.
But now we're left with a bunch of errors. The ones that need fixing immediately are the following:
[W] unknown:7 - file:///usr/share/harbour-webview/qml/harbour-webview.qml:7:30:
Type WebViewPage unavailable
initialPage: Component { WebViewPage { } }
^
[W] unknown:13 - file:///usr/share/harbour-webview/qml/pages/
WebViewPage.qml:13:5: Type WebView unavailable
WebView {
^
[W] unknown:141 - file:///usr/lib64/qt5/qml/Sailfish/WebView/
WebView.qml:141:9: Type TextSelectionController unavailable
TextSelectionController {
^
[W] unknown:14 - file:///usr/lib64/qt5/qml/Sailfish/WebView/Controls/
TextSelectionController.qml:14:1: module "QOfono" is not installed
import QOfono 0.2
^
This cascade of errors all reduces to the last:
[W] unknown:14 - file:///usr/lib64/qt5/qml/Sailfish/WebView/Controls/
TextSelectionController.qml:14:1: module "QOfono" is not installed
import QOfono 0.2
^
The reason for this is also clear. The spec file for sailfish-components-webview makes clear that libqofono 0.117 or above is needed. I don't have this on my system for whatever reason (I'll need to investigate), but to work around this I hacked the spec file so that it wouldn't refuse to install on a system with a lower version, like this:
diff --git a/rpm/sailfish-components-webview.spec
b/rpm/sailfish-components-webview.spec
index 766933ba..c311ebcf 100644
--- a/rpm/sailfish-components-webview.spec
+++ b/rpm/sailfish-components-webview.spec
@@ -18,7 +18,7 @@ Requires: sailfishsilica-qt5 >= 1.1.123
Requires: sailfish-components-media-qt5
Requires: sailfish-components-pickers-qt5
Requires: embedlite-components-qt5 >= 1.21.2
-Requires: libqofono-qt5-declarative >= 0.117
+Requires: libqofono-qt5-declarative >= 0.115
%description
%{summary}.
There's no build-time requirement, so I thought I might get away with it. But clearly not.It seems a bit odd that a text selector component should be requiring an entire separate phone library in order to work. Let's take a look at why.
The ofono code comes at the end of the file. There are two OfonoNetworkRegistration components called cellular1Status and cellular2Status. These represent the state of the two SIM card slots in the device. You might ask why there are only two; can't you have more than two SIM card slots? Well, yes, but I guess this is a problem for future developers to deal with.
These two components feed into the following Boolean value at the top of the code:
readonly property bool _canCall: cellular1Status.registered
|| cellular2Status.registered
Later on in the code we see this being used, like this:
isPhoneNumber = _canCall && _phoneNumberSelected
So what's this all for? When you select some text the browser will present you with some options for what to do with it. Copy to clipboard? Open a link? If it thinks it's a phone number it will offer to make a call to it for you. Unless you don't have a SIM card installed. So that's why libqofono is needed here.You might wonder how it knows it's a phone number at all. The answer to this question isn't in the sailfish-components-webview code. The answer is in embedlite-components, in the SelectionPrototype.js file where we find this code:
_phoneRegex: /^\+?[0-9\s,-.\(\)*#pw]{1,30}$/,
_getSelectedPhoneNumber: function sh_getSelectedPhoneNumber() {
return this._isPhoneNumber(this._getSelectedText().trim());
},
_isPhoneNumber: function sh_isPhoneNumber(selectedText) {
return (this._phoneRegex.test(selectedText) ? selectedText : null);
},
So the decision about whether something is a phone number or not comes down to whether it satisfies the regex /^\+?[0-9\s,-.\(\)*#pw]{1,30}$/ and whether you have a SIM card installed.But that's a bit of a diversion. We only care about this new libqofono. Why is this newer version needed and why don't I have it on my system? Let's find out when and why it was changed.
$ git diff 16ef5cdf44c2eafd7d93e17a41927ef5da700c2b~ \
16ef5cdf44c2eafd7d93e17a41927ef5da700c2b
diff --git a/import/controls/TextSelectionController.qml
b/import/controls/TextSelectionController.qml
index 5c8f2845..71bd83cc 100644
--- a/import/controls/TextSelectionController.qml
+++ b/import/controls/TextSelectionController.qml
@@ -11,7 +11,7 @@
import QtQuick 2.1
import Sailfish.Silica 1.0
-import MeeGo.QOfono 0.2
+import QOfono 0.2
MouseArea {
id: root
diff --git a/rpm/sailfish-components-webview.spec
b/rpm/sailfish-components-webview.spec
index 9a2a3154..5729a8d9 100644
--- a/rpm/sailfish-components-webview.spec
+++ b/rpm/sailfish-components-webview.spec
@@ -18,6 +18,7 @@ Requires: sailfishsilica-qt5 >= 1.1.123
Requires: sailfish-components-media-qt5
Requires: sailfish-components-pickers-qt5
Requires: embedlite-components-qt5 >= 1.21.2
+Requires: libqofono-qt5-declarative >= 0.117
%description
%{summary}.
The import has been updated as have the requirements. But there's been no change to the code, so the libqofono version requirement is probably only needed to deal with the name change of the import.None of this seems essential for ESR 91. My guess is that this change has gone into the development code but hasn't yet made it into a release. So I'm going to hack around it for now (being careful not to commit my hacked changes into the repository).
I've already amended the version number in the spec file, so to get things to work I should just have to reverse this change:
-import MeeGo.QOfono 0.2 +import QOfono 0.2I can do that on-device. This should do it:
sed -i -e 's/QOfono/MeeGo.QOfono/g' \
/usr/lib64/qt5/qml/Sailfish/WebView/Controls/TextSelectionController.qml
Great! That's removed the QML error. But now the app is back to crashing again before it gets to even try to render something on-screen:
$ harbour-webview
[D] unknown:0 - QML debugging is enabled. Only use this in a safe environment.
[...]
UserAgentOverrideHelper app-startup
CONSOLE message:
[JavaScript Error: "Unexpected event profile-after-change" {file:
"resource://gre/modules/URLQueryStrippingListService.jsm" line: 228}]
observe@resource://gre/modules/URLQueryStrippingListService.jsm:228:12
Created LOG for EmbedPrefs
Created LOG for EmbedLiteLayerManager
Segmentation fault
So it's back to the debugger again. But this will have to wait until this evening.[...]
It's the evening and time to put the harbour-webview example through the debugger.
$ gdb harbour-webview
GNU gdb (GDB) Mer (8.2.1+git9)
[...]
Thread 36 "Compositor" received signal SIGSEGV, Segmentation fault.
[Switching to LWP 24061]
mozilla::gl::SwapChain::OffscreenSize (this=0x0)
at gfx/gl/GLScreenBuffer.cpp:129
129 return mPresenter->mBackBuffer->mFb->mSize;
(gdb) bt
#0 mozilla::gl::SwapChain::OffscreenSize (this=0x0)
at gfx/gl/GLScreenBuffer.cpp:129
#1 0x0000007ff3667930 in mozilla::embedlite::EmbedLiteCompositorBridgeParent::
CompositeToDefaultTarget (this=0x7fc4be8da0, aId=...)
at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/UniquePtr.h:290
#2 0x0000007ff12b808c in mozilla::layers::CompositorVsyncScheduler::
ForceComposeToTarget (this=0x7fc4d0c0b0, aTarget=aTarget@entry=0x0,
aRect=aRect@entry=0x0)
at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/layers/
LayersTypes.h:82
#3 0x0000007ff12b80e8 in mozilla::layers::CompositorBridgeParent::
ResumeComposition (this=this@entry=0x7fc4be8da0)
at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/RefPtr.h:313
#4 0x0000007ff12b8174 in mozilla::layers::CompositorBridgeParent::
ResumeCompositionAndResize (this=0x7fc4be8da0, x=<optimized out>,
y=<optimized out>, width=<optimized out>, height=<optimized out>)
at gfx/layers/ipc/CompositorBridgeParent.cpp:794
#5 0x0000007ff12b0d10 in mozilla::detail::RunnableMethodArguments<int, int,
int, int>::applyImpl<mozilla::layers::CompositorBridgeParent, void
(mozilla::layers::CompositorBridgeParent::*)(int, int, int, int),
StoreCopyPassByConstLRef<int>, StoreCopyPassByConstLRef<int>,
StoreCopyPassByConstLRef<int>, StoreCopyPassByConstLRef<int>, 0ul, 1ul,
2ul, 3ul> (args=..., m=<optimized out>, o=<optimized out>)
at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsThreadUtils.h:1151
[...]
#17 0x0000007ff6a0489c in ?? () from /lib64/libc.so.6
(gdb)
This is now a proper crash, not something induced intentionally by the code. Here's the actual code causing the crash taken from GLSCreenBuffer.cpp:
const gfx::IntSize& SwapChain::OffscreenSize() const {
return mPresenter->mBackBuffer->mFb->mSize;
}
The problem here being that the SwapChain object itself is null. So we should look in the calling method to find out what's going on there. Here's the relevant code this time from EmbedLiteCompositorBridgeParent.cpp:
void
EmbedLiteCompositorBridgeParent::CompositeToDefaultTarget(VsyncId aId)
{
GLContext* context = static_cast<CompositorOGL*>(state->mLayerManager->
GetCompositor())->gl();
[...]
if (context->IsOffscreen()) {
MutexAutoLock lock(mRenderMutex);
if (context->GetSwapChain()->OffscreenSize() != mEGLSurfaceSize
&& !context->GetSwapChain()->Resize(mEGLSurfaceSize)) {
return;
}
}
With a bit of digging we can see that the value being returned by context->GetSwapChain() is null:
(gdb) frame 1
#1 0x0000007ff3667930 in mozilla::embedlite::EmbedLiteCompositorBridgeParent::
CompositeToDefaultTarget (this=0x7fc4be8da0, aId=...)
at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/UniquePtr.h:290
290 ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/UniquePtr.h:
No such file or directory.
(gdb) p context
$2 = (mozilla::gl::GLContext *) 0x7ed819ee00
(gdb) p context->GetSwapChain()
Cannot evaluate function -- may be inlined
(gdb) p context.mSwapChain
$3 = {
mTuple = {<mozilla::detail::CompactPairHelper<mozilla::gl::SwapChain*,
mozilla::DefaultDelete<mozilla::gl::SwapChain>,
(mozilla::detail::StorageType)1, (mozilla::detail::StorageType)0>> =
{<mozilla::DefaultDelete<mozilla::gl::SwapChain>> = {<No data fields>},
mFirstA = 0x0}, <No data fields>}}
(gdb) p context.mSwapChain.mTuple
$4 = {<mozilla::detail::CompactPairHelper<mozilla::gl::SwapChain*,
mozilla::DefaultDelete<mozilla::gl::SwapChain>,
(mozilla::detail::StorageType)1, (mozilla::detail::StorageType)0>> =
{<mozilla::DefaultDelete<mozilla::gl::SwapChain>> = {<No data fields>},
mFirstA = 0x0}, <No data fields>}
(gdb) p context.mSwapChain.mTuple.mFirstA
$5 = (mozilla::gl::SwapChain *) 0x0
(gdb)
You may recall that way back in the first three weeks of working on Gecko I hit a problem with the rendering pipeline. The GLScreenBuffer structure that the WebView has been using for a long time had been completely removed and replaced with this SwapChain class.At the time I struggled with how to rearrange the code so that it compiled. I made changes that I couldn't test. And while I did get it to compile, these changes are now coming back to haunt me. Now I need to actually fix this rendering pipeline properly.
There's a bit of me that is glad I'm finally having to do this. I really want to know how it's actually supposed to work.
Clearly the first task will be to figure out why the mSwapChain member of GLContext is never being set. With any luck this will be at the easier end of the difficulty spectrum.
I'm going to try to find where mSwapChain is being — or should be being — set. To do that I'll need to find out where the context is coming from. The context is being passed by CompositorOGL so that would seem to be a good place to start.
Looking through the CompositoryOGL.cpp file we can see that the mGLContext member is being initialised from a value passed in to CompositorOGL::Initialize(). The debugger can help us work back from there.
(gdb) break CompositorOGL::Initialize
Breakpoint 1 at 0x7ff11b0c3c: file gfx/layers/opengl/CompositorOGL.cpp,
line 380.
(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /usr/bin/harbour-webview
[...]
Thread 36 "Compositor" hit Breakpoint 1, mozilla::layers::CompositorOGL::
Initialize (this=0x7ee0002f50, out_failureReason=0x7f1faac520)
at gfx/layers/opengl/CompositorOGL.cpp:380
380 bool CompositorOGL::Initialize(nsCString* const out_failureReason) {
(gdb)
Ah! This is interesting. It's not being passed in because there are two different overloads of the CompositorOGL::Initialize() method and the code is using the other one. In this other piece of code the context is created directly:
bool CompositorOGL::Initialize(nsCString* const out_failureReason) {
ScopedGfxFeatureReporter reporter("GL Layers");
// Do not allow double initialization
MOZ_ASSERT(mGLContext == nullptr || !mOwnsGLContext,
"Don't reinitialize CompositorOGL");
if (!mGLContext) {
MOZ_ASSERT(mOwnsGLContext);
mGLContext = CreateContext();
}
[...]
Let's see what happens with the context creation.
Thread 36 "Compositor" hit Breakpoint 5, mozilla::layers::CompositorOGL::
CreateContext (this=this@entry=0x7ee0002f50)
at gfx/layers/opengl/CompositorOGL.cpp:227
227 already_AddRefed<mozilla::gl::GLContext> CompositorOGL::CreateContext() {
(gdb) n
231 nsIWidget* widget = mWidget->RealWidget();
(gdb)
232 void* widgetOpenGLContext =
(gdb)
234 if (widgetOpenGLContext) {
(gdb)
248 if (!context && gfxEnv::LayersPreferOffscreen()) {
(gdb) b GLContextProviderEGL::CreateHeadless
Breakpoint 6 at 0x7ff1133740: file gfx/gl/GLContextProviderEGL.cpp, line 1245.
(gdb) c
Continuing.
Thread 36 "Compositor" hit Breakpoint 6, mozilla::gl::GLContextProviderEGL::
CreateHeadless (desc=..., out_failureId=out_failureId@entry=0x7f1faed1c8)
at gfx/gl/GLContextProviderEGL.cpp:1245
1245 const GLContextCreateDesc& desc, nsACString* const out_failureId) {
(gdb) n
1246 const auto display = DefaultEglDisplay(out_failureId);
(gdb)
1247 if (!display) {
(gdb) p display
$8 = std::shared_ptr<mozilla::gl::EglDisplay> (use count 1, weak count 2)
= {get() = 0x7ee0004cb0}
(gdb) n
1250 mozilla::gfx::IntSize dummySize = mozilla::gfx::IntSize(16, 16);
(gdb) b GLContextEGL::CreateEGLPBufferOffscreenContext
Breakpoint 7 at 0x7ff11335b8: file gfx/gl/GLContextProviderEGL.cpp, line 1233.
(gdb) c
Continuing.
Thread 36 "Compositor" hit Breakpoint 7, mozilla::gl::GLContextEGL::
CreateEGLPBufferOffscreenContext (
display=std::shared_ptr<mozilla::gl::EglDisplay> (use count 2, weak count 2)
= {...}, desc=..., size=...,
out_failureId=out_failureId@entry=0x7f1faed1c8)
at gfx/gl/GLContextProviderEGL.cpp:1233
1233 const mozilla::gfx::IntSize& size, nsACString* const
out_failureId) {
(gdb) b GLContextEGL::CreateEGLPBufferOffscreenContextImpl
Breakpoint 8 at 0x7ff1133160: file gfx/gl/GLContextProviderEGL.cpp, line 1185.
(gdb) c
Continuing.
Thread 36 "Compositor" hit Breakpoint 8, mozilla::gl::GLContextEGL::
CreateEGLPBufferOffscreenContextImpl (
egl=std::shared_ptr<mozilla::gl::EglDisplay> (use count 3, weak count 2) =
{...}, desc=..., size=..., useGles=useGles@entry=false,
out_failureId=out_failureId@entry=0x7f1faed1c8)
at gfx/gl/GLContextProviderEGL.cpp:1185
1185 nsACString* const out_failureId) {
(gdb) n
1186 const EGLConfig config = ChooseConfig(*egl, desc, useGles);
(gdb)
1187 if (config == EGL_NO_CONFIG) {
(gdb)
1193 if (GLContext::ShouldSpew()) {
(gdb)
1197 mozilla::gfx::IntSize pbSize(size);
(gdb)
1307 include/c++/8.3.0/bits/shared_ptr_base.h: No such file or directory.
(gdb)
1208 if (!surface) {
(gdb)
1214 auto fullDesc = GLContextDesc{desc};
(gdb)
1215 fullDesc.isOffscreen = true;
(gdb)
1217 egl, fullDesc, config, surface, useGles, out_failureId);
(gdb) b GLContextEGL::CreateGLContext
Breakpoint 9 at 0x7ff1132548: file gfx/gl/GLContextProviderEGL.cpp, line 618.
(gdb) c
Continuing.
Thread 36 "Compositor" hit Breakpoint 9, mozilla::gl::GLContextEGL::
CreateGLContext (egl=std::shared_ptr<mozilla::gl::EglDisplay>
(use count 4, weak count 2) = {...}, desc=...,
config=config@entry=0x55558fc450, surface=surface@entry=0x7ee0008f40,
useGles=useGles@entry=false, out_failureId=out_failureId@entry=0x7f1faed1c8)
at gfx/gl/GLContextProviderEGL.cpp:618
618 nsACString* const out_failureId) {
(gdb) n
621 std::vector<EGLint> required_attribs;
(gdb)
We're getting down into the depths now. It's surprisingly thrilling to be seeing this code again. I recall that this GLContextEGL::CreateGLContext() method is where a lot of the action happens.But my head is full and this feels like a good place to leave things. Inside this method might be the right place to initialise mSwapChain, but it's definitely not happening here.
Tomorrow I'll do a sweep of the other code to check whether any attempt is being made to initialise it somewhere else. If not I'll add in some initialisation code to see what happens.
If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.
Comments
Uncover Disqus comments