Closed Bug 1007604 Opened 7 years ago Closed 7 years ago

do not use nsVoidArray in nsViewManager

Categories

(Core :: Web Painting, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: Swatinem, Assigned: Swatinem)

References

Details

Attachments

(2 files)

No description provided.
Attachment #8419328 - Flags: review?(roc)
https://hg.mozilla.org/integration/mozilla-inbound/rev/f40a93e56161

-  int32_t index;
+  uint32_t index;
   for (index = 0; index < gViewManagers->Length(); index++) {
     nsViewManager* vm = gViewManagers->ElementAt(index);

Sorry about that :-(
https://hg.mozilla.org/mozilla-central/rev/4eabc2a942a3
https://hg.mozilla.org/mozilla-central/rev/f40a93e56161
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Comment on attachment 8419328 [details] [diff] [review]
viewmanagerarray.patch

Review of attachment 8419328 [details] [diff] [review]:
-----------------------------------------------------------------

::: view/src/nsViewManager.cpp
@@ +1093,5 @@
>  {
>    NS_PRECONDITION(IsRootVM(), "Must be root VM for this to be called!");
>  
>    int32_t index;
> +  for (index = 0; index < gViewManagers->Length(); index++) {

Do you need to null-check gViewManagers here first? The old code effectively did.
(In reply to Jonas Sicking (:sicking) from comment #4)
> Do you need to null-check gViewManagers here first? The old code effectively
> did.

Good catch.

Although I’m not sure that will ever happen. As long as there is a `nsViewManager` alive, `gViewManagers` should always exist. Unless something very weird is happening?
Attachment #8420399 - Flags: review?(jonas)
Comment on attachment 8420399 [details] [diff] [review]
nullcheckviewmanager.patch

Review of attachment 8420399 [details] [diff] [review]:
-----------------------------------------------------------------

r=me either way.

::: view/src/nsViewManager.cpp
@@ +1092,5 @@
>  nsViewManager::CallWillPaintOnObservers()
>  {
>    NS_PRECONDITION(IsRootVM(), "Must be root VM for this to be called!");
>  
> +  if (NS_WARN_IF(gViewManagers == nullptr)) {

I prefer !gViewManagers. I even think that's in our coding guidelines.
Attachment #8420399 - Flags: review?(jonas) → review+
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.