Merge code-review-fixes

erfansamandarian · Jan 15, 2018 · 2ef46d2 · 2ef46d2
2 parents fa911ca + ce42adb
commit 2ef46d2
Show file tree

Hide file tree

Showing 8 changed files with 89 additions and 46 deletions.
diff --git a/autoupdater.cpp b/autoupdater.cpp
@@ -44,7 +44,8 @@ void AutoUpdater::checkForUpdates(QString endpoint) {
 void AutoUpdater::updateFromVersionDesc(QUrl versionDesc, QByteArray base64Sig) {
     if (inProgress) return;
     inProgress = true;
-    QMetaObject::invokeMethod(this, "updateFromVersionDescPerform", Qt::QueuedConnection, Q_ARG(QUrl, versionDesc), Q_ARG(QByteArray, base64Sig));
+    QMetaObject::invokeMethod(this, "updateFromVersionDescPerform", Qt::QueuedConnection, Q_ARG(QUrl, versionDesc),
+                              Q_ARG(QByteArray, base64Sig));
 }
 void AutoUpdater::abort() {
     QMetaObject::invokeMethod(this, "abortPerform", Qt::QueuedConnection);
@@ -265,9 +266,12 @@ void AutoUpdater::startNextDownload() {
     // WARNING: TODO: do we want to make a separate dir inside tempPath? ; we should ensure downloadFile always overrides
     QString dest = QDir::tempPath() + QDir::separator() + url.fileName();
 
-    // Check if the download is already downloaded - could happen if we try to do a full upgrade when we've already prepared one
+    // Check if the download is already downloaded - could happen if we try to do a full upgrade when we've
+    // already prepared one
     // Sketchy case: if the file does not exist, getFileChecksum would return the default sha256 hash; - 
-    //   this would actually prevent a case where the version descriptor is generated from empty files from breaking the system - because this check would return true, and then the file wouldn't exist at all, emitting an error (this shouldn't be able to happen, but still...)
+    //   this would actually prevent a case where the version descriptor is generated from empty files from breaking
+    // the system - because this check would return true, and then the file wouldn't exist at all, emitting an error
+    // (this shouldn't be able to happen, but still...)
     if (checksum == getFileChecksum(dest)) {
         preparedFiles.push_back(dest);
         startNextDownload();
@@ -324,7 +328,8 @@ void AutoUpdater::downloadFinished()
 // ABORT
 void AutoUpdater::abortPerform() {
     // EXPLANATION: those will be aborted, and then in the 'finished' handler, they will be deleted via ->deleteLater()
-    // since the event handlers are executed in the event loop, one might think calling .checkForVer() right after .abort() will re-set currentCheck before the 'finished' handler is executed
+    // since the event handlers are executed in the event loop, one might think calling .checkForVer() right after
+    // .abort() will re-set currentCheck before the 'finished' handler is executed
     // This is not a problem, because all public methods call the internal ones with invokeMethod and queuedConnection
     if (currentCheck) currentCheck->abort();
     if (currentDownload) currentDownload->abort();
@@ -337,4 +342,4 @@ void AutoUpdater::abortPerform() {
     output.close();
 
     inProgress = false;
-}
+}
diff --git a/images/stremio.icns b/images/stremio.icns
diff --git a/main.cpp b/main.cpp
@@ -95,7 +95,8 @@ int main(int argc, char **argv)
     engine.load(QUrl(QStringLiteral("qrc:/main.qml")));
 
     QObject::connect( &app, &SingleApplication::receivedMessage, &app, &MainApp::processMessage );
-    QObject::connect( &app, SIGNAL(receivedMessage(QVariant, QVariant)), engine.rootObjects().value(0), SLOT(onAppMessageReceived(QVariant, QVariant)) );
+    QObject::connect( &app, SIGNAL(receivedMessage(QVariant, QVariant)), engine.rootObjects().value(0),
+                      SLOT(onAppMessageReceived(QVariant, QVariant)) );
 
     return app.exec();
 }
diff --git a/main.qml b/main.qml
@@ -30,15 +30,17 @@ ApplicationWindow {
     color: "#201f32";
     title: appTitle
 
-    // This is built on the assumption it will be executed twice, at the start and end of the loading stage; which means everything has to be checked
+    // This is built on the assumption it will be executed twice, at the start and end of the loading stage;
+    // which means everything has to be checked
     property string injectedJs: "initShellComm()"
 
     // Transport
     QtObject {
         id: transport
         readonly property string shellVersion: Qt.application.version
         property string serverAddress: "http://127.0.0.1:11470" // will be set to something else if server inits on another port
-        readonly property bool isFullscreen: root.visibility === Window.FullScreen // just to send the initial state
+
+	readonly property bool isFullscreen: root.visibility === Window.FullScreen // just to send the initial state
 
         signal event(var ev, var args)
         function onEvent(ev, args) {
@@ -51,7 +53,8 @@ ApplicationWindow {
             if (ev === "open-external") Qt.openUrlExternally(args)
             if (ev === "balloon-show" && root.notificationsEnabled) trayIcon.showMessage(args.title, args.content)
             if (ev === "win-focus") { if (!root.visible) root.show(); root.raise(); root.requestActivate(); }
-            if (ev === "win-set-visibility") root.visibility = args.hasOwnProperty('fullscreen') ? (args.fullscreen ? Window.FullScreen : Window.Windowed) : args.visibility
+            if (ev === "win-set-visibility") root.visibility = args.hasOwnProperty('fullscreen') ?
+                                             (args.fullscreen ? Window.FullScreen : Window.Windowed) : args.visibility
             if (ev === "autoupdater-notif-clicked" && autoUpdater.onNotifClicked) autoUpdater.onNotifClicked()
             if (ev === "chroma-toggle") { args.enabled ? chroma.enable() : chroma.disable() }
             if (ev === "screensaver-toggle") shouldDisableScreensaver(args.disabled)
@@ -84,7 +87,7 @@ ApplicationWindow {
     }
 
     function shouldDisableScreensaver(condition) {
-        if (condition === screenSaver.disabled) return
+        if (condition === screenSaver.disabled) return;
         condition ? screenSaver.disable() : screenSaver.enable();
         screenSaver.disabled = condition;
     }
@@ -149,7 +152,8 @@ ApplicationWindow {
         id: screenSaver
         property bool disabled: false // track last state so we don't call it multiple times
     }
-    // This is needed so that 300s after the remote control has been used, we can re-enable the screensaver (if the player is not playing)
+    // This is needed so that 300s after the remote control has been used, we can re-enable the screensaver
+    // (if the player is not playing)
     Timer {
         id: timerScreensaver
         interval: 300000
@@ -172,7 +176,8 @@ ApplicationWindow {
     //
     Process {
         id: streamingServer
-        property string errMessage: "Error while starting streaming server. Please consider re-installing Stremio from https://www.stremio.com"
+        property string errMessage:
+            "Error while starting streaming server. Please consider re-installing Stremio from https://www.stremio.com"
         property int errors: 0
         property bool fastReload: false
 
@@ -199,9 +204,11 @@ ApplicationWindow {
             transport.event("server-address", address)
         }
         onErrorThrown: function (error) {
-            if (streamingServer.fastReload && error == 1) return; // inhibit errors during fast reload mode; we'll unset that after we've restarted the server 
+            if (streamingServer.fastReload && error == 1) return; // inhibit errors during fast reload mode;
+                                                                  // we'll unset that after we've restarted the server
             errorDialog.text = streamingServer.errMessage
-            errorDialog.detailedText = 'Stremio streaming server has thrown an error \nQProcess::ProcessError code: ' + error
+            errorDialog.detailedText =
+                    'Stremio streaming server has thrown an error \nQProcess::ProcessError code: ' + error
             errorDialog.visible = true
        }
     }
@@ -265,23 +272,27 @@ ApplicationWindow {
 
         onLoadingChanged: function(loadRequest) {
             if (webView.tries > 0) {
-                // show the webview only if we're already on the backupUrl; the first one (network based) can fail because of many reasons, including captive portals
+                // show the webview only if we're already on the backupUrl; the first one (network based)
+                // can fail because of many reasons, including captive portals
                 splashScreen.visible = false
             }
 
             if (loadRequest.status == WebEngineView.LoadSucceededStatus) { 
                 webView.webChannel.registerObject( 'transport', transport );
 
-                // Try-catch to be able to return the error as result, but still throw it in the client context so it can be caught and reported
-                var injectedJS = "try { "+root.injectedJs+" } catch(e) { setTimeout(function() { throw e }); e.message || JSON.stringify(e) }";
+                // Try-catch to be able to return the error as result, but still throw it in the client context
+                // so it can be caught and reported
+                var injectedJS = "try { "+root.injectedJs+" } " +
+                        "catch(e) { setTimeout(function() { throw e }); e.message || JSON.stringify(e) }";
                 webView.runJavaScript(injectedJS, function(err) {
                     if (! err) {
                         splashScreen.visible = false
                         webView.tries = 0
                         return
                     }
 
-                    errorDialog.text = "Error while applying shell JS. Please consider re-installing Stremio from https://www.stremio.com"
+                    errorDialog.text = "Error while applying shell JS." +
+                            " Please consider re-installing Stremio from https://www.stremio.com"
                     errorDialog.detailedText = err
                     errorDialog.visible = true
 
@@ -295,7 +306,8 @@ ApplicationWindow {
                 });
             }
 
-            var shouldRetry = loadRequest.status == WebEngineView.LoadFailedStatus || loadRequest.status == WebEngineView.LoadStoppedStatus
+            var shouldRetry = loadRequest.status == WebEngineView.LoadFailedStatus ||
+                    loadRequest.status == WebEngineView.LoadStoppedStatus
             if ( shouldRetry && webView.tries < webView.maxTries) {
                 retryTimer.restart()
             }
@@ -367,8 +379,10 @@ ApplicationWindow {
             SequentialAnimation {
                 id: pulseOpacity
                 running: true
-                NumberAnimation { target: splashLogo; property: "opacity"; to: 1.0; duration: 600; easing.type: Easing.Linear; }
-                NumberAnimation { target: splashLogo; property: "opacity"; to: 0.3; duration: 600; easing.type: Easing.Linear; }
+                NumberAnimation { target: splashLogo; property: "opacity"; to: 1.0; duration: 600;
+                    easing.type: Easing.Linear; }
+                NumberAnimation { target: splashLogo; property: "opacity"; to: 0.3; duration: 600;
+                    easing.type: Easing.Linear; }
                 loops: Animation.Infinite
             }
         }
@@ -393,7 +407,8 @@ ApplicationWindow {
     }
 
     onVisibilityChanged: {
-        transport.event("win-visibility-changed", { visible: root.visible, visibility: root.visibility, isFullscreen: root.visibility === Window.FullScreen })
+        transport.event("win-visibility-changed", { visible: root.visible, visibility: root.visibility,
+                            isFullscreen: root.visibility === Window.FullScreen })
     }
 
     property int appState: Qt.application.state;
@@ -420,11 +435,13 @@ ApplicationWindow {
     signal autoUpdaterErr(var msg, var err);
     signal autoUpdaterRestartTimer();
     function initAutoUpdater() {
-        var endpoints = ["https://www.strem.io/updater/check", "https://www.stremio.com/updater/check", "https://www.stremio.net/updater/check"];
+        var endpoints = ["https://www.strem.io/updater/check", "https://www.stremio.com/updater/check",
+                         "https://www.stremio.net/updater/check"];
         var fallbackSite = "https://www.stremio.com/?fromFailedAutoupdate=true";
         var doAutoupdate = autoUpdater.isInstalled()
 
-        // On Linux, because we use AppImage, we cannot do partial updates - because we can't replace files in the read-only mountpoint
+        // On Linux, because we use AppImage, we cannot do partial updates - because we can't replace files
+        // in the read-only mountpoint
         if (Qt.platform.os === "linux" && doAutoupdate) autoUpdater.setForceFullUpdate(true);
 
         var args = Qt.application.arguments
@@ -462,7 +479,8 @@ ApplicationWindow {
         // Re-start this timer only from the main thread
         root.autoUpdaterRestartTimer.connect(function() { autoUpdaterLongTimer.restart() })
 
-        // WARNING: all of the slot handlers are handled in another thread, that's why we need the autoUpdaterErr() signal - to bring execution back to UI thread
+        // WARNING: all of the slot handlers are handled in another thread, that's why we need the autoUpdaterErr()
+        // signal - to bring execution back to UI thread
         autoUpdater.checkFinished.connect(function(check) {
             // reset the notif, so there's no chance we'd trigger a re-start while downloading new ver
             if (check && !check.upToDate) autoUpdater.onNotifClicked = null;
@@ -482,8 +500,9 @@ ApplicationWindow {
             autoUpdaterLongTimer.restart()
 
             // Fallback to auto-update: just open the website
-            // Ideally, we don't want this happening if the check has failed, because in most cases of the check, we would not have a new version
-            // however, since the check will only happen if we have an internet connection according to QNetworkConfigurationManager, checks should not fail
+            // Ideally, we don't want this happening if the check has failed, because in most cases of the check,
+            // we would not have a new version however, since the check will only happen if we have an internet
+            // connection according to QNetworkConfigurationManager, checks should not fail
             // Apply the fall-back only if there's no prepared version before...
             if (! autoUpdater.onNotifClicked) {
                 transport.queueEvent("autoupdater-show-notif", { mode: "fallback" })
@@ -498,7 +517,8 @@ ApplicationWindow {
 
             console.log("Auto-updater: prepared update "+preparedFiles.join(", "))
 
-            // When we finish preparing an update, we must call transport.queueEvent so that the app can receive a notification event once it loads
+            // When we finish preparing an update, we must call transport.queueEvent so that the app can receive
+            // a notification event once it loads
             // Then, we must set .onNotifClicked to what we'll do when the notification is clicked
 
             if (preparedFiles.length == 2) {
@@ -531,8 +551,10 @@ ApplicationWindow {
                     +"&& NEW=/Applications/$(date +%s).app"
                     +"&& MNT=\"/Volumes/Stremio "+ver+"\""
 
-                    +"&& hdiutil attach \"$DMG\" -nobrowse -noautoopen" // NOTE: this returns 0, even if it's already mounted
-                    //+"&& MNT=$(hdiutil attach \"$DMG\" -nobrowse -noautoopen | awk -F'/Volumes/' '/Apple_HFS/ {print $2}') &&" // WARNING: I'm not sure about this working on every OSX ver
+                    +"&& hdiutil attach \"$DMG\" -nobrowse -noautoopen" // NOTE: this returns 0,
+                                                                        // even if it's already mounted
+                    //+"&& MNT=$(hdiutil attach \"$DMG\" -nobrowse -noautoopen | awk -F'/Volumes/' '/Apple_HFS/ {print $2}') &&"
+                            // WARNING: I'm not sure about this working on every OSX ver
 
                     +"&& cp -R \"$MNT\"/*.app \"$NEW\""
                     +"&& rm -rf /Applications/Stremio.app && mv \"$NEW\" \"/Applications/Stremio.app\""
@@ -567,26 +589,30 @@ ApplicationWindow {
                 console.log("Auto-updater: executing Linux update");
 
                 var baseName = firstFile.split("/").pop()
-                var code = autoUpdater.executeCmd("/bin/sh", ["-c", "mv '"+firstFile+"' $HOME; chmod +x $HOME/'"+baseName+"'"], false)
+                var code = autoUpdater.executeCmd("/bin/sh",
+                                                  ["-c", "mv '"+firstFile+"' $HOME; chmod +x $HOME/'"+baseName+"'"], false)
                 if (code !== 0) {
                     root.autoUpdaterErr("preparing Linux .appimage failed", null);
                     return;
                 }
                 transport.queueEvent("autoupdater-show-notif", { mode: "launchNew" })
                 autoUpdater.onNotifClicked = function() {
-                    autoUpdater.executeCmd("/bin/sh", ["-c", "$HOME/'"+baseName+"'"], true) // crappy, but otherwise we have to write code to get env var
+                    autoUpdater.executeCmd("/bin/sh", ["-c", "$HOME/'"+baseName+"'"], true)
+                                    // crappy, but otherwise we have to write code to get env var
                     Qt.quit()
                 }
             } else {
                 root.autoUpdaterErr("Insane auto-update: "+preparedFiles.join(", "), null)
             }
 
-            // WARNING: this seems to randomly crash the program in rare cases if called more than once in this signal handler...
+            // WARNING: this seems to randomly crash the program in rare cases if called more than once
+            // in this signal handler...
             root.autoUpdaterRestartTimer()
         })
     }
     // Explanation: when the long timer expires, we schedule the short timer; we do that, 
-    // because in case the computer has been asleep for a long time, we want another short timer so we don't check immediately (network not connected yet, etc)
+    // because in case the computer has been asleep for a long time, we want another short timer so we don't check
+    // immediately (network not connected yet, etc)
     // we also schedule the short timer if the computer is offline
     Timer {
         id: autoUpdaterLongTimer
@@ -605,7 +631,8 @@ ApplicationWindow {
     // On complete handler
     //
     Component.onCompleted: function() {
-        // Kind of hacky way to ensure there are no Qt bindings going on; otherwise when we go to fullscreen Qt tries to restore original window size
+        // Kind of hacky way to ensure there are no Qt bindings going on; otherwise when we go to fullscreen
+        // Qt tries to restore original window size
         root.height = root.initialHeight
         root.width = root.initialWidth
 
@@ -617,7 +644,8 @@ ApplicationWindow {
             launchServer();
 
         // Handle file opens
-        var lastArg = args[1]; // not actually last, but we want to be consistent with what happens when we open a second instance (main.cpp)
+        var lastArg = args[1]; // not actually last, but we want to be consistent with what happens when we open
+                               // a second instance (main.cpp)
         if (args.length > 1 && !lastArg.match('^--')) onAppOpenMedia(lastArg)
 
         // Check for updates

diff --git a/razerchroma.cpp b/razerchroma.cpp
@@ -48,7 +48,8 @@ void RazerChroma::enable() {
 void RazerChroma::disable() {
 #ifdef _WIN32
 if (m_ChromaSDKImpl) {
-	// WARNING: after this is done, effects do not get applied on the next enable() even though we use a separate instance that initialized successfully
+    // WARNING: after this is done, effects do not get applied on the next enable() even though we use a separate instance
+    // that initialized successfully
 	m_ChromaSDKImpl->ShowColor(KEYBOARD_DEVICES, RGB(140, 40, 170));
 	//m_ChromaSDKImpl->UnInitialize();
 	//delete m_ChromaSDKImpl;