381474 - Fix NS_OBSERVER_ARRAY_NOTIFY_OBSERVERS not to take obstype_ parameter

Status: REOPENED

(Core :: XPCOM, enhancement, P5)

Description

[Olli Pettay [:smaug][[e-mail]]]

See https://bugzilla.mozilla.org/show_bug.cgi?id=380674#c4

Comment 1

[Rahul Gandhi]

Sir i want to take this bug i am well acquainted with the knowledge of c/c++ and would give my best effort to solve this bug.

Comment 2

[Benjamin Smedberg]

Excellent. I *think* that a typedef as mentioned in that bug will basically make the parameter unnecessary. Please let me know if you have any questions.

Comment 3

[Rahul Gandhi]

So we have to Fix NS_OBSERVER_ARRAY_NOTIFY_OBSERVERS not to take obstype_ parameter. So From where to begin?


(In reply to Benjamin Smedberg  [:bsmedberg] from comment #2)
> Excellent. I *think* that a typedef as mentioned in that bug will basically
> make the parameter unnecessary. Please let me know if you have any questions.

Comment 4

[Rahul Gandhi]

Attachment: Fix NS_OBSERVER_ARRAY_NOTIFY_OBSERVERS not to take obstype_ parameter

Comment 5

[Josh Matthews [:jdm]]

MXR (http://mxr.mozilla.org/mozilla-central/search?string=ns_observer_array) suggests this isn't resolved. Rahul's working on some other bugs right now.

Comment 6

[Peiyong Lin[:lpy](UTC+8)]

https://bugzilla.mozilla.org/show_bug.cgi?id=380674#c4 The idea is using array_::elem_type, but when I tried to add |typedef T elem_type| in nsTObserverArray, I could not figure out where should I put |typename T| to get compiled. Any help please? Thank you.

Comment 7

[Peiyong Lin[:lpy](UTC+8)]

After spending whole night looking up C++ rules, I still couldn't find a solution to solve this. So bsmedberg, could you please figure it out? Or is it possible to get rid of |obstype_| parameter? Thank you.

Comment 8

[Reuben Morais [:reuben]]

As mentioned on IRC, you should be able to do:

  decltype(array_)::ObserverType obs_;

Make sure you include "mozilla/Types.h".

Also, adding |typename| is only needed when you do something like this:

 typedef typename T::InnerType ObserverType;

In order to differentiate between an inner type and a static member of T, for example.

Comment 9

[Peiyong Lin[:lpy](UTC+8)]

push to try https://tbpl.mozilla.org/?tree=Try&rev=23b7cfcef2cc
Thank you :)

Comment 10

[Peiyong Lin[:lpy](UTC+8)]

Hi Reuben, the patch couldn't be built under "Windows XP Opt", "B2G Desktop Windows Opt", "B2G ICS Emulator Opt". Any help? The patch is here https://pastebin.mozilla.org/4112500

Comment 11

[Benjamin Smedberg]

Windows error: c:\builds\moz2_slave\try-w32-0000000000000000000000\build\obj-firefox\dist\include\nsNodeUtils.h(119) : error C2039: 'elem_type' : is not a member of '`global namespace''

From this code: https://hg.mozilla.org/try/annotate/23b7cfcef2cc/content/base/src/nsNodeUtils.h#l119
through this macro: https://hg.mozilla.org/try/rev/23b7cfcef2cc#l3.94

Which basically looks like MSVC is for some reason expanding "decltype()" to the empty string.

I cannot explain this yet...

Comment 12

[Georg Fritzsche [:gfritzsche]]

(In reply to Peiyong Lin[:lpy](UTC+8) from comment #10)
> Hi Reuben, the patch couldn't be built under "Windows XP Opt", "B2G Desktop
> Windows Opt", "B2G ICS Emulator Opt". Any help? The patch is here
> https://pastebin.mozilla.org/4112500

The pastebin is gone, can you attach the patch to the bug here?