Do not show busy dialog if progress dialog is active while calling a plugin #14225

AkariDN · 2018-07-25T14:35:36Z

Description

Applies only for calling plugins from the main thread. If execution of a plugin takes some time then CGUIDialogBusy automatically opens. But if we already have topmost modal CGUIDialogProgress then additional busy dialog is no longer needed - we can use progress dialog's render loop and handle "cancel" action. Especially in case of sequentially plugin calling.

Motivation and Context

There is no need to show two status dialogs simultaneously.

How Has This Been Tested?

Build and functionality tested on Windows and Android.

Types of change

Bug fix (non-breaking change which fixes an issue)
Improvement (non-breaking change which improves existing functionality)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Cosmetic change (non-breaking change that doesn't touch code)
None of the above (please explain below)

Checklist:

My code follows the Code Guidelines of this project
My change requires a change to the documentation, either Doxygen or wiki
I have updated the documentation accordingly
I have read the Contributing document
I have added tests to cover my change
All new and existing tests passed

xbmc/dialogs/GUIDialogProgress.h

@@ -48,6 +48,7 @@ class CGUIDialogProgress :
   \return true if the dialog is closed, false if the user cancels early.
   */
  bool Wait(int progresstime = 10);
+  bool WaitOnEvent(CEvent& event, int progresstime = 10);


AkariDN · 2018-07-25T15:26:11Z

Thanks for clarifying. Fixed.

xbmc/dialogs/GUIDialogProgress.cpp

+  return WaitOnEvent(m_done, progresstime);
+}
+
+bool CGUIDialogProgress::WaitOnEvent(CEvent& event, int progresstime /*= 10*/)


xbmc/dialogs/GUIDialogProgress.h

@@ -48,6 +48,7 @@ class CGUIDialogProgress :
   \return true if the dialog is closed, false if the user cancels early.
   */
  bool Wait(int progresstime = 10);
+  bool WaitOnEvent(CEvent& event, int progresstime);


xbmc/filesystem/PluginDirectory.cpp

-      if (!CGUIDialogBusy::WaitOnEvent(m_fetchComplete, 200))
-      {
+      CGUIWindowManager& wm = CServiceBroker::GetGUI()->GetWindowManager();
+      CGUIDialogProgress* progress = wm.IsModalDialogTopmost(WINDOW_DIALOG_PROGRESS) ? wm.GetWindow<CGUIDialogProgress>(WINDOW_DIALOG_PROGRESS) : nullptr;


xbmc/dialogs/GUIDialogProgress.cpp

+
+bool CGUIDialogProgress::WaitOnEvent(CEvent& event, int progresstime)
+{
+  while (!event.WaitMSec(progresstime) && m_active && !m_bCanceled)


xbmc/filesystem/PluginDirectory.cpp

      }
+      else if (!CGUIDialogBusy::WaitOnEvent(m_fetchComplete, 200))
+        m_cancelled = true;
      scriptObs.Abort();


AkariDN · 2018-07-25T20:58:03Z

Thank you for detailed explanations. Fixed.

hudokkow · 2018-07-25T21:40:57Z

Jenkins commands only work for team members. This build already shows some failures. I'll start a new build, if failures are not code related.

Thank you for your contributions, btw! 👍

Rechi · 2018-07-25T21:44:41Z

For some reason the command works for non team members too.
No platform failed in that jenkins run yet.

hudokkow · 2018-07-25T22:00:33Z

Weird! It never worked for non-team members before. And of course, you're right, all is good. First run failed on some platforms but re-run shows all is well. Great work @AkariDN

…plugin

AkariDN · 2018-07-26T09:49:59Z

@FernetMenta according to your remarks I would like to keep old code unchanged and set 1ms blocking interval only to new function. And should I handle closing of progress dialog without canceling? This should not happen in normal mode but somehow dialog can be closed e.g. from other thread.

FernetMenta · 2018-07-26T12:55:18Z

new code looks good to me

FernetMenta · 2018-07-26T15:29:05Z

jenkins build this please

FernetMenta reviewed Jul 25, 2018

View reviewed changes

AkariDN force-pushed the busydlg branch 2 times, most recently from cc2a8f4 to 8f22b1e Compare July 25, 2018 15:23

FernetMenta reviewed Jul 25, 2018

View reviewed changes

xbmc/dialogs/GUIDialogProgress.cpp Outdated

return WaitOnEvent(m_done, progresstime);

}

bool CGUIDialogProgress::WaitOnEvent(CEvent& event, int progresstime /*= 10*/)

This comment was marked as spam.

Sign in to view

FernetMenta reviewed Jul 25, 2018

View reviewed changes

AkariDN force-pushed the busydlg branch from 8f22b1e to f95ed1b Compare July 25, 2018 15:32

garbear reviewed Jul 25, 2018

View reviewed changes

AkariDN force-pushed the busydlg branch 2 times, most recently from 095a60a to 620dffa Compare July 25, 2018 18:19

garbear reviewed Jul 25, 2018

View reviewed changes

xbmc/dialogs/GUIDialogProgress.cpp Outdated

bool CGUIDialogProgress::WaitOnEvent(CEvent& event, int progresstime)

{

while (!event.WaitMSec(progresstime) && m_active && !m_bCanceled)

This comment was marked as spam.

Sign in to view

garbear reviewed Jul 25, 2018

View reviewed changes

xbmc/filesystem/PluginDirectory.cpp Outdated

}

else if (!CGUIDialogBusy::WaitOnEvent(m_fetchComplete, 200))

m_cancelled = true;

scriptObs.Abort();

This comment was marked as spam.

Sign in to view

AkariDN force-pushed the busydlg branch from 620dffa to 4c07b52 Compare July 25, 2018 20:36

Do not show busy dialog if progress dialog is active while calling a …

5c3a515

…plugin

AkariDN force-pushed the busydlg branch from 4c07b52 to 5c3a515 Compare July 26, 2018 09:48

FernetMenta merged commit 9ec10b9 into xbmc:master Jul 26, 2018

Rechi added Type: Improvement non-breaking change which improves existing functionality v18 Leia labels Jul 26, 2018

Rechi added this to the Leia 18.0-alpha3 milestone Jul 26, 2018

enen92 mentioned this pull request Jun 27, 2020

PluginDirectory: Refactor to remove the dependency on GUI (concurrent busy dialogs) #18104

Closed

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not show busy dialog if progress dialog is active while calling a plugin #14225

Do not show busy dialog if progress dialog is active while calling a plugin #14225

AkariDN commented Jul 25, 2018

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

AkariDN commented Jul 25, 2018

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

AkariDN commented Jul 25, 2018

hudokkow commented Jul 25, 2018

Rechi commented Jul 25, 2018

hudokkow commented Jul 25, 2018

AkariDN commented Jul 26, 2018

FernetMenta commented Jul 26, 2018

FernetMenta commented Jul 26, 2018

Do not show busy dialog if progress dialog is active while calling a plugin #14225

Do not show busy dialog if progress dialog is active while calling a plugin #14225

Conversation

AkariDN commented Jul 25, 2018

Description

Motivation and Context

How Has This Been Tested?

Types of change

Checklist:

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

AkariDN commented Jul 25, 2018

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

AkariDN commented Jul 25, 2018

hudokkow commented Jul 25, 2018

Rechi commented Jul 25, 2018

hudokkow commented Jul 25, 2018

AkariDN commented Jul 26, 2018

FernetMenta commented Jul 26, 2018

FernetMenta commented Jul 26, 2018