[PUSHED] Re: [PATCH] fdo#38837: Timers must end eventually [Writer idle timer]

[Lubos Lunak]

On Friday 14 of December 2012, pkoroau pkoroau wrote:
> 2012/12/12 Caolán McNamara :
> > Could you submit a new patch with the fixups ?
> >
> > C.
>
> Sure! Thank you for the review.

 Looks like this has been pushed yet, so done, thanks for the patch.

-- 
 Lubos Lunak
 l.lu...@suse.cz
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [PATCH] fdo#38837: Timers must end eventually [Writer idle timer]

[pkoroau pkoroau]

2012/12/12 Caolán McNamara :
> Could you submit a new patch with the fixups ?
>
> C.
Sure! Thank you for the review.

pkoroau
--
diff --git a/sw/inc/IDocumentTimerAccess.hxx b/sw/inc/IDocumentTimerAccess.hxx
index 84e3cd4..458ad79 100644
--- a/sw/inc/IDocumentTimerAccess.hxx
+++ b/sw/inc/IDocumentTimerAccess.hxx
@@ -20,31 +20,41 @@
  #ifndef IDOCUMENTTIMERACCESS_HXX_INCLUDED
  #define IDOCUMENTTIMERACCESS_HXX_INCLUDED
 
- /** Get information about the current document state
+ /** Manipulate background jobs of the document. It starts with a mode of
+ 'started' and a block count of 0.
  */
  class IDocumentTimerAccess
  {
  public:
 /**
-Set modus to start, i.e. start timer if block count == 0
+Set modus to 'start'.
 */
 virtual void StartIdling() = 0;
 
 /**
-Set modus to stopped, i.e. stop timer if running
+Set mode to 'stopped'.
 */
 virtual void StopIdling() = 0;
 
 /**
-Increment block count, stop timer if running
+Increment block count.
 */
 virtual void BlockIdling() = 0;
 
 /**
-Decrement block count, start timer if block count == 0 AND modus == start
+Decrement block count.
 */
 virtual void UnblockIdling() = 0;
 
+/**
+Do these jobs asynchronously: do grammar checking,
+do layout, and update fields.
+They will be delayed until mode is start AND block count == 0.
+The implementation might delay them further, for example
+it might wait until the application is idle.
+*/
+virtual void StartBackgroundJobs() = 0;
+
  protected:
 virtual ~IDocumentTimerAccess() {};
  };
diff --git a/sw/inc/doc.hxx b/sw/inc/doc.hxx
index ae910d8..51d669c 100644
--- a/sw/inc/doc.hxx
+++ b/sw/inc/doc.hxx
@@ -972,6 +972,7 @@ public:
 virtual void StopIdling();
 virtual void BlockIdling();
 virtual void UnblockIdling();
+virtual void StartBackgroundJobs();
 
 /** IDocumentChartDataProviderAccess
 */
diff --git a/sw/source/core/doc/docfld.cxx b/sw/source/core/doc/docfld.cxx
index 18953ad..7f6e6ac 100644
--- a/sw/source/core/doc/docfld.cxx
+++ b/sw/source/core/doc/docfld.cxx
@@ -2564,8 +2564,8 @@ void SwDocUpdtFld::RemoveFldType( const SwFieldType& 
rType )
 }
 }
 
-SwDocUpdtFld::SwDocUpdtFld()
-: pFldSortLst(0), nFldLstGetMode(0)
+SwDocUpdtFld::SwDocUpdtFld(SwDoc* pDoc)
+: pFldSortLst(0), nFldLstGetMode(0), pDocument(pDoc)
 {
 bInUpdateFlds = bFldsDirty = sal_False;
 memset( aFldTypeTable, 0, sizeof( aFldTypeTable ) );
diff --git a/sw/source/core/doc/doclay.cxx b/sw/source/core/doc/doclay.cxx
index 06b10d5..c4079c5 100644
--- a/sw/source/core/doc/doclay.cxx
+++ b/sw/source/core/doc/doclay.cxx
@@ -1826,6 +1826,11 @@ void SwDoc::UnblockIdling()
 aIdleTimer.Start();
 }
 
+void SwDoc::StartBackgroundJobs() {
+// Trigger DoIdleJobs(), asynchronously.
+aIdleTimer.Start();
+}
+
 /*
 |*
 |*  SwDoc::DoIdleJobs()
@@ -1846,11 +1851,10 @@ IMPL_LINK( SwDoc, DoIdleJobs, Timer *, pTimer )
 ViewShell *pSh, *pStartSh;
 pSh = pStartSh = GetCurrentViewShell();
 do {
 if( pSh->ActionPend() )
 {
-if( pTimer )
-pTimer->Start();
+pTimer->Start();
 return 0;
 }
 pSh = (ViewShell*)pSh->GetNext();
 } while( pSh != pStartSh );
@@ -1865,28 +1869,35 @@ IMPL_LINK( SwDoc, DoIdleJobs, Timer *, pTimer )
 if (bIsOnlineSpell && bIsAutoGrammar)
 StartGrammarChecking( *this );
 }
-SwFldUpdateFlags nFldUpdFlag;
 std::set aAllLayouts = GetAllLayouts();//swmod 080320
 std::set::iterator pLayIter = aAllLayouts.begin();
 for ( ;pLayIter != aAllLayouts.end();++pLayIter )
 {
 if ((*pLayIter)->IsIdleFormat())
 {
 (*pLayIter)->GetCurrShell()->LayoutIdle();
-break;
+
+// Defer the remaining work.
+pTimer->Start();
+return 0;
 }
 }
-bool bAllValid = pLayIter == aAllLayouts.end() ? 1 : 0;
-if( bAllValid && ( AUTOUPD_FIELD_ONLY ==
- ( nFldUpdFlag = getFieldUpdateFlags(true) )
+
+SwFldUpdateFlags nFldUpdFlag = getFieldUpdateFlags(true);
+if( ( AUTOUPD_FIELD_ONLY == nFldUpdFlag
 || AUTOUPD_FIELD_AND_CHARTS == nFldUpdFlag ) &&
-GetUpdtFlds().IsFieldsDirty() &&
-!GetUpdtFlds().IsInUpdateFlds() &&
-!IsExpFldsLocked()
+GetUpdtFlds().IsFieldsDirty()
 // If we switch the field name the Fields are not updated.
 // So the "backgorund update" should always be carried out
 /* && !pStartSh->GetViewOptions()->IsFldName()*/ )
 {
+if ( GetUpdtFlds().IsInUpdateFlds() ||
+ IsExpFldsLocked() )
+   

Re: [PATCH] fdo#38837: Timers must end eventually [Writer idle timer]

[Caolán McNamara]

On Sat, 2012-12-08 at 21:17 +0100, pkoroau pkoroau wrote:
> 2012/12/5 Caolán McNamara:
> > 1. In e.g. SetNeedGrammarCheck and SetFieldsDirty shouldn't
> > StartBackgroundJobs only be called for the "true" case and not the
> > "false" case ?
> 
>  I planned to negate. That should be:
> 
> if ( GetUpdtFlds().IsInUpdateFlds() ||
>  IsExpFldsLocked() )

Could you submit a new patch with the fixups ?

C.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [PATCH] fdo#38837: Timers must end eventually [Writer idle timer]

[pkoroau pkoroau]

2012/12/5 Caolán McNamara:
> 1. In e.g. SetNeedGrammarCheck and SetFieldsDirty shouldn't
> StartBackgroundJobs only be called for the "true" case and not the
> "false" case ?

1. Yes, that's right.


>  if ( !GetUpdtFlds().IsInUpdateFlds() &&
>   !IsExpFldsLocked() )

2. I planned to negate. That should be:

if ( GetUpdtFlds().IsInUpdateFlds() ||
 IsExpFldsLocked() )


3. IDocumentTimerAccess's patched class-level documentation reads:
It starts with a mode of 'stopped'
That's wrong, it should be:
It starts with a mode of 'start'


--
pkoroau

2012/12/5 Caolán McNamara :
> On Wed, 2012-11-28 at 23:25 +0100, pkoroau pkoroau wrote:
>> Hello, I'm sending this patch to fix Writer's aIdleTimer.
>
> This looks promising. But a couple of things.
>
> 1. In e.g. SetNeedGrammarCheck and SetFieldsDirty shouldn't
> StartBackgroundJobs only be called for the "true" case and not the
> "false" case ?
>
> 2. the...
>
>  if ( !GetUpdtFlds().IsInUpdateFlds() &&
>   !IsExpFldsLocked() )
>  {
>  pTimer->Start();
>  return 0;
>  }
>
> condition to me looks that it will restart the timer if we not currently
> updating-fields but the updating-fields flag is set after that condition
> and the updating fields works done there. So it suggests that fields
> will never be actually updated and the timer run again and again.
>
> Maybe that should be...
>
> if (GetUpdtFlds().IsInUpdateFlds())
> {
> //Already updating fields, try again later
> pTimer->Start();
> return 0;
> }
>
> ?
>
> C.
>
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [PATCH] fdo#38837: Timers must end eventually [Writer idle timer]

[Caolán McNamara]

On Wed, 2012-11-28 at 23:25 +0100, pkoroau pkoroau wrote:
> Hello, I'm sending this patch to fix Writer's aIdleTimer.

This looks promising. But a couple of things.

1. In e.g. SetNeedGrammarCheck and SetFieldsDirty shouldn't
StartBackgroundJobs only be called for the "true" case and not the
"false" case ?

2. the...

 if ( !GetUpdtFlds().IsInUpdateFlds() &&
  !IsExpFldsLocked() )
 {
 pTimer->Start();
 return 0;
 }

condition to me looks that it will restart the timer if we not currently
updating-fields but the updating-fields flag is set after that condition
and the updating fields works done there. So it suggests that fields
will never be actually updated and the timer run again and again.

Maybe that should be...

if (GetUpdtFlds().IsInUpdateFlds())
{
//Already updating fields, try again later
pTimer->Start();
return 0;
}

?

C.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice