Keep Qt related command line parameters during restart

RESOLVED FIXED

Status

Core Graveyard
Widget: Qt
RESOLVED FIXED
8 years ago
2 years ago

People

(Reporter: Steffen Imhof, Assigned: Steffen Imhof)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

8 years ago
Created attachment 446465 [details] [diff] [review]
Patch to store all Qt parameters for later use

If the browser restarts itself for example because extensions were enabled/disabled, it drops all command line parameters given previously.

This is not good for those that are Qt related for example specifying the graphics system with "-graphicssystem raster" gets lost after the restart which might lead to unexpected effects.
(Assignee)

Updated

8 years ago
Attachment #446465 - Flags: review?(dougt)
Assignee: nobody → steffen.imhof
Status: NEW → ASSIGNED
Hardware: x86 → All

Comment 1

8 years ago
Comment on attachment 446465 [details] [diff] [review]
Patch to store all Qt parameters for later use

rob, can you take a look at this?
Attachment #446465 - Flags: review?(dougt) → review?(rstrong)
Comment on attachment 446465 [details] [diff] [review]
Patch to store all Qt parameters for later use

>                             PRBool aBlankCommandLine = PR_FALSE)
> {
>   aNative->Quit(); // release DDE mutex, if we're holding it
> 
>   // Restart this process by exec'ing it into the current process
>   // if supported by the platform.  Otherwise, use NSPR.
>  
>   if (aBlankCommandLine) {
>+#if defined(MOZ_WIDGET_QT)
>+    // Remove only arguments not given to Qt
>+    gRestartArgc = gQtOnlyArgc;
>+    gRestartArgv = gQtOnlyArgv;
>+    gRestartArgv[gRestartArgc] = nsnull;

This last line seems unnecessary when you're setting the last element to null when initialising gQtOnlyArgv.

> #if defined(MOZ_WIDGET_QT)
>     QApplication app(gArgc, gArgv);
>+
>+    QStringList nonQtArguments = app.arguments();
>+    gQtOnlyArgc = 1;
>+    gQtOnlyArgv = (char**) malloc(sizeof(char*) * (gRestartArgc - nonQtArguments.size() + 1));
>+
>+    // copy binary path
>+    gQtOnlyArgv[0] = gRestartArgv[0];
>+
>+    for (int i = 1; i < gRestartArgc; ++i) {
>+      if (!nonQtArguments.contains(gRestartArgv[i])) {
>+        // copy arguments used by Qt for later
>+        gQtOnlyArgv[gQtOnlyArgc++] = gRestartArgv[i];
>+      }
>+    }
>+    gQtOnlyArgv[gQtOnlyArgc] = 0;

Use nsnull please.

r=me with those changes
Attachment #446465 - Flags: review?(rstrong) → review+
Depends on: 568786
No longer depends on: 568786
(Assignee)

Comment 3

8 years ago
Created attachment 449241 [details] [diff] [review]
Updated patch

Updated version of the patch that addresses the review issues, also it is refreshed to the latest mozilla-central code and the memory allocation is made bigger, because it was too small.
http://hg.mozilla.org/mozilla-central/rev/bcc82f4858c5
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.