1435637 - Minimally cleanup nsRefreshDriver.

Status: RESOLVED FIXED

(Core :: Layout, enhancement)

Description

[Emilio Cobos Álvarez (:emilio)]

Just use more ranged loops on locals and such.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1435637: Minimally tidy nsRefreshDriver.

All the arrays we're switching to ranged loops can't mutate during the loop
since are locals or not referenced from other places.

Review commit: https://reviewboard.mozilla.org/r/217784/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/217784/

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8948284 [details]
Bug 1435637: Minimally tidy nsRefreshDriver.

https://reviewboard.mozilla.org/r/217784/#review223508

::: layout/base/nsRefreshDriver.cpp:1863
(Diff revision 1)
>  
>    // Resize events should be fired before layout flushes or
>    // calling animation frame callbacks.
>    AutoTArray<nsIPresShell*, 16> observers;
>    observers.AppendElements(mResizeEventFlushObservers);
> -  for (uint32_t i = observers.Length(); i; --i) {
> +  for (nsIPresShell* shell : observers) {

This needs to use Reversed.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8948284 [details]
Bug 1435637: Minimally tidy nsRefreshDriver.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/217784/diff/1-2/

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8948284 [details]
Bug 1435637: Minimally tidy nsRefreshDriver.

https://reviewboard.mozilla.org/r/217784/#review223510

::: layout/base/nsRefreshDriver.cpp:1641
(Diff revision 2)
>    // Hold all AnimationEventDispatcher in mAnimationEventFlushObservers as
>    // a RefPtr<> array since each AnimationEventDispatcher might be destroyed
>    // during processing the previous dispatcher.
> -  size_t len = mAnimationEventFlushObservers.Length();
>    AutoTArray<RefPtr<AnimationEventDispatcher>, 16> dispatchers;
> -  RefPtr<AnimationEventDispatcher>* elems = dispatchers.AppendElements(len);
> +  dispatchers.AppendElements(mAnimationEventFlushObservers);

Oh, I didn't know that AppendElements works between RefPtr<> and raw pointer.

::: layout/base/nsRefreshDriver.cpp:1975
(Diff revision 2)
>    }
>  
>  #ifdef MOZ_XUL
>    // Update any popups that may need to be moved or hidden due to their
>    // anchor changing.
> -  nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
> +  if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) {

Though this isn't related to this bug, I am OK with this. :)

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b3319e229c7d
Minimally tidy nsRefreshDriver. r=hiro

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/b3319e229c7d