After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 769546 - Captive portal helper started for gdm
Captive portal helper started for gdm
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: portal-helper
3.21.x
Other Linux
: Urgent normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2016-08-05 11:15 UTC by Vít Ondruch
Modified: 2017-06-28 00:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
network: Don't start portal helper under GDM (978 bytes, patch)
2017-02-19 15:39 UTC, Bastien Nocera
committed Details | Review
portalHelper: Refuse to start under GDM (1003 bytes, patch)
2017-02-19 15:39 UTC, Bastien Nocera
needs-work Details | Review

Description Vít Ondruch 2016-08-05 11:15:04 UTC
Description of problem:
When captive portal is detected, there is displayed login window, but without content. Looking at the list of processes, I see two processes hanging around, loading my system:

  PID USER      PR  NI    VIRT    RES    SHR S  %CPU %MEM     TIME+ COMMAND                                                                                                                                        
17832 gdm       20   0 2281580  73876  56952 S  77,6  0,6  10:11.41 WebKitWebProces                                                                                                                                
 8642 gdm       20   0 2769492  81068  65344 S  42,4  0,7   5:49.88 gnome-shell-por      

They seems to be stuck.


Version-Release number of selected component (if applicable):
$ rpm -qf /usr/libexec/gnome-shell-portal-helper
gnome-shell-3.21.4-1.fc25.x86_64

$ rpm -qf /usr/libexec/webkit2gtk-4.0/WebKitWebProcess
webkitgtk4-2.13.4-1.fc26.x86_64


How reproducible:
Always


Steps to Reproduce:
1. Connect to WiFi with captive portal.
2. The login dialog is displayed without content => no way how to log in.
3. The processes mentioned above are still hanging around

Actual results:
The login dialog does not work and some processes are left around.


Expected results:
The login dialog works just fine.


Additional info:
Just FTR, I am running Wayland session if that makes any difference.
Comment 1 Rui Matos 2016-08-17 14:46:53 UTC
I think this is https://bugzilla.redhat.com/show_bug.cgi?id=1347436 . Do other webkit apps (e.g. yelp) work?
Comment 2 Vít Ondruch 2016-08-24 20:35:23 UTC
(In reply to Rui Matos from comment #1)
Unfortunately hard to tell, since I updated my system. The only think I can test ATM is that yelp seems to work just fine.
Comment 3 Bastien Nocera 2017-02-19 15:29:17 UTC
The portal helper seems to be started for each and every session, *including* the gdm session.

I'll repurpose this bug for that.
Comment 4 Bastien Nocera 2017-02-19 15:39:33 UTC
Created attachment 346190 [details] [review]
network: Don't start portal helper under GDM
Comment 5 Bastien Nocera 2017-02-19 15:39:45 UTC
Created attachment 346191 [details] [review]
portalHelper: Refuse to start under GDM

Just in case our precautions not to start the helper fail, this would
throw an error in the system logs.
Comment 6 Bastien Nocera 2017-02-19 15:40:38 UTC
Both untested patches.
Comment 7 Florian Müllner 2017-02-19 16:45:24 UTC
Review of attachment 346190 [details] [review]:

I didn't test either, but the patch looks obviously correct
Comment 8 Florian Müllner 2017-02-19 16:45:38 UTC
Review of attachment 346191 [details] [review]:

::: js/portalHelper/main.js
@@ +367,3 @@
     Gettext.textdomain(Config.GETTEXT_PACKAGE);
 
+    if (GLib.getenv('RUNNING_UNDER_GDM') != null) {

Just to make sure: Do we know that this variable will be set? I see in gdm-launch-environment.c that it's pushed into the gnome-session environment, but does it apply to D-Bus activated processes as well?

@@ +368,3 @@
 
+    if (GLib.getenv('RUNNING_UNDER_GDM') != null) {
+        GLib.error('gnome-shell-portal-helper should never run under the login screen');

You are probably thinking of g_error(), but that's not available under introspection - use either

  throw new Error('gnome-shell-portal-helper should never run under the login screen');

(but that'll also log a stacktrace) or plain

  log('gnome-shell-portal-helper ...');
Comment 9 Bastien Nocera 2017-02-22 15:22:38 UTC
(In reply to Florian Müllner from comment #8)
> Review of attachment 346191 [details] [review] [review]:
> 
> ::: js/portalHelper/main.js
> @@ +367,3 @@
>      Gettext.textdomain(Config.GETTEXT_PACKAGE);
>  
> +    if (GLib.getenv('RUNNING_UNDER_GDM') != null) {
> 
> Just to make sure: Do we know that this variable will be set? I see in
> gdm-launch-environment.c that it's pushed into the gnome-session
> environment, but does it apply to D-Bus activated processes as well?

No idea. Ray?

> @@ +368,3 @@
>  
> +    if (GLib.getenv('RUNNING_UNDER_GDM') != null) {
> +        GLib.error('gnome-shell-portal-helper should never run under the
> login screen');
> 
> You are probably thinking of g_error(), but that's not available under
> introspection - use either
> 
>   throw new Error('gnome-shell-portal-helper should never run under the
> login screen');
> 
> (but that'll also log a stacktrace) or plain
> 
>   log('gnome-shell-portal-helper ...');

The earlier one might be better for our case. It's a sanity-check.
Comment 10 Bastien Nocera 2017-02-22 15:23:29 UTC
Comment on attachment 346190 [details] [review]
network: Don't start portal helper under GDM

Attachment 346190 [details] pushed as 607b2ef - network: Don't start portal helper under GDM
Comment 11 Ray Strode [halfline] 2017-02-22 16:30:22 UTC
(In reply to Bastien Nocera from comment #9)
> (In reply to Florian Müllner from comment #8)
> > Review of attachment 346191 [details] [review] [review] [review]:
> > 
> > ::: js/portalHelper/main.js
> > @@ +367,3 @@
> >      Gettext.textdomain(Config.GETTEXT_PACKAGE);
> >  
> > +    if (GLib.getenv('RUNNING_UNDER_GDM') != null) {
> > 
> > Just to make sure: Do we know that this variable will be set? I see in
> > gdm-launch-environment.c that it's pushed into the gnome-session
> > environment, but does it apply to D-Bus activated processes as well?
> 
> No idea. Ray?
gnome-session pushes the environment up to dbus, so should be safe.  I think a more "right" approach, though, would be to check the logind session type of the caller.
Comment 12 Michael Catanzaro 2017-05-18 20:13:45 UTC
Can this bug be closed?
Comment 13 Bastien Nocera 2017-05-18 21:43:02 UTC
(In reply to Michael Catanzaro from comment #12)
> Can this bug be closed?

Ideally, somebody would rework my last patch to make sure the portal helper backs out even if called under GDM, but I don't really have the time to implement and test. If not calling it out is good enough, then I'm fine with closing it.
Comment 14 Michael Catanzaro 2017-06-28 00:08:32 UTC
(In reply to Bastien Nocera from comment #13)
> Ideally, somebody would rework my last patch to make sure the portal helper
> backs out even if called under GDM, but I don't really have the time to
> implement and test. If not calling it out is good enough, then I'm fine with
> closing it.

That seems like a good robustness enhancement for another bug report, but I think the original problem is solved, so let's close this. It's confusing to keep bug reports open for improvement once the main issue has already been fixed.