1046841 - Convert rest of xpcom/ to Gecko style

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Biru [:poiru]]

See bug 995730 and bug 1022456.

Comment 1

[Biru [:poiru]]

Attachment: Fix indentation in xpcom/components/

Comment 2

[Biru [:poiru]]

Attachment: Fix style violations in xpcom/components/

Comment 3

[Biru [:poiru]]

Attachment: Fix argument names in xpcom/components/

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8465547 [details] [diff] [review]
Fix indentation in xpcom/components/

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

::: xpcom/components/nsComponentManager.cpp
@@ +427,5 @@
> +  if (aModule->mCategoryEntries) {
> +    const mozilla::Module::CategoryEntry* entry;
> +    for (entry = aModule->mCategoryEntries; entry->category; ++entry)
> +      nsCategoryManager::GetSingleton()->
> +      AddCategoryEntry(entry->category,

This isn't the correct indentation.  Maybe introduce a temporary variable for GetSingleton here and hoist it out of the loop?

Comment 5

[Nathan Froyd [:froydnj]]

Comment on attachment 8465548 [details] [diff] [review]
Fix style violations in xpcom/components/

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

::: xpcom/components/ManifestParser.cpp
@@ +300,2 @@
>      if (testdata.Equals(aValue))
>        aResult = comparison ? eOK : eBad;

Actually, forgot to ask: are you fixing up bracing in all these files, too?  Because there are...well, a lot of places that need fixed.

Comment 6

[Biru [:poiru]]

Attachment: Brace one-line control statements in xpcom/components/

(In reply to Nathan Froyd (:froydnj) from comment #5)
> Comment on attachment 8465548 [details] [diff] [review]
> Fix style violations in xpcom/components/
> 
> Review of attachment 8465548 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: xpcom/components/ManifestParser.cpp
> @@ +300,2 @@
> >      if (testdata.Equals(aValue))
> >        aResult = comparison ? eOK : eBad;
> 
> Actually, forgot to ask: are you fixing up bracing in all these files, too? 
> Because there are...well, a lot of places that need fixed.

Yup, here you go.

Comment 7

[Nathan Froyd [:froydnj]]

Comment on attachment 8466262 [details] [diff] [review]
Brace one-line control statements in xpcom/components/

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

Great, thank you!

Comment 8

[Biru [:poiru]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ce357b802532

Try push: https://tbpl.mozilla.org/?tree=Try&rev=12c029f7de77

Comment 9

[Ryan VanderMeulen [:RyanVM]]

(In reply to Birunthan Mohanathas [:poiru] from comment #8)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/ce357b802532
> 
> Try push: https://tbpl.mozilla.org/?tree=Try&rev=12c029f7de77

Sorry, I had to back this out due to having some major merge conflicts with bug 977026 that I wasn't comfortable trying to rebase around on the fly :(

https://hg.mozilla.org/integration/mozilla-inbound/rev/b31b3ee2b42f

Comment 10

[Biru [:poiru]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #9)
> (In reply to Birunthan Mohanathas [:poiru] from comment #8)
> > https://hg.mozilla.org/integration/mozilla-inbound/rev/ce357b802532
> > 
> > Try push: https://tbpl.mozilla.org/?tree=Try&rev=12c029f7de77
> 
> Sorry, I had to back this out due to having some major merge conflicts with
> bug 977026 that I wasn't comfortable trying to rebase around on the fly :(

No worries! Rebased and relanded:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5a957b1c510

Try push: https://tbpl.mozilla.org/?tree=Try&rev=259ccfc19547

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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

Comment 12

[Biru [:poiru]]

Attachment: Fix indentation in xpcom/build/

Comment 13

[Biru [:poiru]]

Attachment: Fix indentation in xpcom/build/

Comment 14

[Biru [:poiru]]

Attachment: Fix style violations in xpcom/build/

Comment 15

[Biru [:poiru]]

Attachment: Fix argument names and return types in xpcom/build/

Comment 16

[Biru [:poiru]]

Attachment: Brace one-line control statements in xpcom/build/

Comment 17

[Nathan Froyd [:froydnj]]

Comment on attachment 8471867 [details] [diff] [review]
Fix style violations in xpcom/build/

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

::: xpcom/build/IOInterposer.cpp
@@ +224,5 @@
>        newLists->mStageObservers.push_back(aObserver);
>      }
>      mObserverLists = newLists;
> +    mObservedOperations = (IOInterposeObserver::Operation)(
> +      mObservedOperations | aOp);

I think this looks less unusual if formatted like:

    mObservedOperations =
      (IOInterposeObserver::Operation)(mObservedOperations | aOp);

@@ +245,5 @@
>      if (aOp & IOInterposeObserver::OpCreateOrOpen) {
>        VectorRemove(newLists->mCreateObservers, aObserver);
>        if (newLists->mCreateObservers.empty()) {
> +        mObservedOperations = (IOInterposeObserver::Operation)(
> +          mObservedOperations & ~IOInterposeObserver::OpCreateOrOpen);

...and similarly for this and all similar operations below, which can be done like so:

        mObservedOperations =
          (IOInterposeObserver::Operation)(mObservedOperations &
                                           ~IOInterposeObserver::OpRead);

::: xpcom/build/perfprobe.cpp
@@ +197,4 @@
>    }
>    ULONG result =
>      RegisterTraceGuids(&ControlCallback
> +                       /*RequestAddress: Sets mSessions appropriately.*/,

This is such bizarre formatting.

Comment 18

[Biru [:poiru]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4399b34c5259

Try push: https://tbpl.mozilla.org/?tree=Try&rev=de6ae17ec90f

Comment 19

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4399b34c5259

Comment 20

[Biru [:poiru]]

Attachment: Fix more style violations in xpcom/**.cpp

Comment 21

[Biru [:poiru]]

Attachment: Fix comment style in nsCOMPtr.h

Comment 22

[Biru [:poiru]]

Attachment: Fix more style violations in xpcom/**.h

Comment 23

[Biru [:poiru]]

Attachment: Fix indentation in xpcom/

Comment 24

[Biru [:poiru]]

Attachment: Fix style violations in xpcom/

Comment 25

[Biru [:poiru]]

Attachment: Fix argument names in xpcom/

Comment 26

[Nathan Froyd [:froydnj]]

Comment on attachment 8478068 [details] [diff] [review]
Fix comment style in nsCOMPtr.h

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

::: xpcom/glue/nsCOMPtr.h
@@ +115,5 @@
>  
>  } // namespace mozilla
>  
> +/**
> + * Cooperates with nsCOMPtr to allow you to assign in a pointer _without_

I think it's worth saying "already_AddRefed cooperates with..." for clarity.

@@ +333,5 @@
>  do_QueryInterface(already_AddRefed<T>&)
>  {
>    // This signature exists solely to _stop_ you from doing the bad thing.
> +  // Saying |do_QueryInterface()| on a pointer that is not otherwise owned by
> +  // someone else is an automatic leak. See <http://bugzilla.mozilla.org/show_bug.cgi?id=8221>.

Optional: just 'bug 8221' here would be sufficient.

@@ +342,5 @@
>  do_QueryInterface(already_AddRefed<T>&, nsresult*)
>  {
>    // This signature exists solely to _stop_ you from doing the bad thing.
> +  // Saying |do_QueryInterface()| on a pointer that is not otherwise owned by
> +  // someone else is an automatic leak. See <http://bugzilla.mozilla.org/show_bug.cgi?id=8221>.

Likewise.

@@ +409,5 @@
> +/**
> + * Factors implementation for all template versions of nsCOMPtr.
> + *
> + * This should really be an |nsCOMPtr<nsISupports>|, but this wouldn't work
> + * because unlike the

I don't think this bit adds anything; it should be deleted.

@@ +411,5 @@
> + *
> + * This should really be an |nsCOMPtr<nsISupports>|, but this wouldn't work
> + * because unlike the
> + *
> + * Here's the way people normally do things like this

Nit: ':' at the end of this sentence.

@@ +1354,5 @@
>    nsCOMPtr<nsISupports>& mTargetSmartPtr;
>  };
>  
> +// Used around a |nsCOMPtr| when
> +// ...makes the class |nsGetterAddRefs<T>| invisible.

This comment makes no sense; I'd just delete it.

Comment 27

[Nathan Froyd [:froydnj]]

Comment on attachment 8478067 [details] [diff] [review]
Fix more style violations in xpcom/**.cpp

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

Please fold these patches as appropriate prior to landing.

::: xpcom/io/nsLocalFileWin.cpp
@@ +566,5 @@
>  // of NSPR so that fields of |PRFilePrivate| #ifdef'd by them are not copied.
>  // Similarly, |_MDFileDesc| is taken from nsprpub/pr/include/md/_win95.h.
>  // In an unlikely case we switch to 'NT'-dependent NSPR AND this temporary
>  // workaround last beyond the switch, |PRFilePrivate| and |_MDFileDesc|
>  // need to be changed to match the definitions for WinNT.

Maybe someday some brave soul will investigate whether we still need all this garbage.

::: xpcom/threads/ThreadStackHelper.cpp
@@ +362,5 @@
>    virtual ~ThreadContext() {}
>  
> +  virtual uint64_t GetBase() const { return uint64_t(mStackBase); }
> +  virtual uint32_t GetSize() const { return mStackSize; }
> +  virtual bool GetMemoryAtAddress(uint64_t address, uint8_t*  value) const

Nit: extra space before |value|.

@@ +368,3 @@
>      return GetMemoryAtAddressInternal(address, value);
>    }
> +  virtual bool GetMemoryAtAddress(uint64_t address, uint16_t*  value) const

Here too.

@@ +372,3 @@
>      return GetMemoryAtAddressInternal(address, value);
>    }
> +  virtual bool GetMemoryAtAddress(uint64_t address, uint32_t*  value) const

Here too.

@@ +376,3 @@
>      return GetMemoryAtAddressInternal(address, value);
>    }
> +  virtual bool GetMemoryAtAddress(uint64_t address, uint64_t*  value) const

Here too.

Comment 28

[Biru [:poiru]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b373c1d4e8ae
https://hg.mozilla.org/integration/mozilla-inbound/rev/db05fccb028c
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe4f8a3cdb19
https://hg.mozilla.org/integration/mozilla-inbound/rev/7afb7c612b5a

Try push: https://tbpl.mozilla.org/?tree=Try&rev=ed7967e9724d

Comment 29

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b373c1d4e8ae
https://hg.mozilla.org/mozilla-central/rev/db05fccb028c
https://hg.mozilla.org/mozilla-central/rev/fe4f8a3cdb19
https://hg.mozilla.org/mozilla-central/rev/7afb7c612b5a

Comment 30

[Biru [:poiru]]

Closing this. I'll file separate bugs for the remaining bits (mainly tests and macros).