What's the purpose of Test::renderShellClientAndWaitForShown?

I guess the reason why createShellSurface doesn't have CreationSetup parameter is because of wl_shell, but without it we would need custom "createShellSurface" helpers in some tests, e.g. TestShellClient::testUserSetFullscreenXdgShell.

I guess the reason why createShellSurface doesn't have CreationSetup parameter is because of wl_shell,

Exactly, having low level control doesn't make sense if you're also trying to abstract.
You have to handle the configure yourself, and you can't do that with a QObject.

I'd be up for adding an XdgShellSurface *createXdgShellSurface(ShellSurfaceType, CreationSetup) which doesn't include WlShell

In D18583#401516, @davidedmundson wrote:

I'd be up for adding an XdgShellSurface *createXdgShellSurface(ShellSurfaceType, CreationSetup) which doesn't include WlShell

Yep, makes sense.

I have several comments, feel free to ignore them until this revision is good enough for landing:

• it would be better to just call blockRequestGeometry and unblockRequestGeometry instead of using RAII because we don't actually benefit from it. Also, we have a field that is used only once, during initialization;
• some of my frame callback comments are still not addressed, e.g. initXdgShellSurface and testQuickTiling. This can fire back at us in some tests;
• would it make sense to add testXdgInitiallyMinimized as well?
• given what RequestGeometryBlocker is doing, could you please add a todo item to rename it to ConfigureBlocker? We would address it after wl-shell support is dropped.

As far as I understand, we'll stick with the current approach. Emitting surfaceCreated after the surface is committed is out of scope, right?

I don't want to use AbstractClient::blockGeometryUpdates as it doesn't just call configure when it's done, it goes into setGeometry which is subtly different, especially for the code path where toplevels should choose their own size.

I could refactor ShellClient::RequestGeometryBlocker to call into some new method on ShellClient when it destructs, which finishInit could then call directly.
I'm not really convinced it's better, as we're probably still going to want some field to know we're in this state.

As far as I understand, we'll stick with the current approach. Emitting surfaceCreated after the surface is committed is out of scope, right?

Yeah, I think that would introduce more problems than it solves and we'd probably still end up needing a geometryblocker to process all the different initial states happening in different orders.

One compromise maybe worth considering is putting a signal in ShellClient emitted after the first surface commit.

I'm not really convinced it's better, as we're probably still going to want some field to know we're in this state.

In which state?

Looks good to me, though I'd prefer to have blockRequestGeometry method

I don't think it adds a lot unless it's as a matching block/unblock pair of methods, and I deliberately refactored requestGeometry for the sole reason of having it easily re-usable for finishInit and the Blocker destructor.

I've made the other two changes. Lets revisit this one when we do that rename.