1313470 - Convert XPCOM test TestCOMPtr to a gtest

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Eric Rahm [:erahm]]

Convert xpcom/tests/TestCOMPtr.cpp to a gtest and move to xpcom/tests/gtest/.

Comment 1

[Eric Rahm [:erahm]]

Attachment: Part 0: Cleanup indentation

This is just whitespace changes with a few minor deletions.

MozReview-Commit-ID: A7fpHLqt5fU

Comment 2

[Eric Rahm [:erahm]]

Attachment: Part 1: Split out test functions

This just splits out test functions, no logic is changed.

MozReview-Commit-ID: EjGYyUjf1b2

Comment 3

[Eric Rahm [:erahm]]

Attachment: Part 2: Convert TestCOMPtr to a gtest

MozReview-Commit-ID: HfcLTmvkRc8

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8806112 [details] [diff] [review]
Part 0: Cleanup indentation

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

Please delete trailing whitespace while you are here.

Comment 5

[Eric Rahm [:erahm]]

(In reply to Nathan Froyd [:froydnj] from comment #4)
> Comment on attachment 8806112 [details] [diff] [review]
> Part 0: Cleanup indentation
> 
> Review of attachment 8806112 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Please delete trailing whitespace while you are here.

The offending line gets deleted later in the patchset.

Comment 6

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e7d5b8135ae61384a51c0b909e5eb2aac9e6e035
Bug 1313470 - Part 0: Cleanup indentation. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/f012923cfd8b903bc342910ec81cff508624906a
Bug 1313470 - Part 1: Split out test functions. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/00f7b342bb29c4f26c7e78fade11b631054aadc4
Bug 1313470 - Part 2: Convert TestCOMPtr to a gtest. r=froydnj

Comment 7

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/561b87e98adacbd47769a71a0e6c72c5dcf66a9d for crashing on OS X, https://treeherder.mozilla.org/logviewer.html#?job_id=38640567&repo=mozilla-inbound.

Comment 8

[Eric Rahm [:erahm]]

I can't repro this locally with an OSX debug build.

Relevant log snippet:

> 21:06:34     INFO -  TEST-START | COMPtr.Bloat_Smart
> 21:06:34     INFO -  [2276] ###!!! ASSERTION: QueryInterface needed: 'query_result.get() == mRawPtr', file /builds/slave/m-in-m64-d-0000000000000000000/build/src/obj-firefox/dist/include/nsCOMPtr.h, line 414
> 21:07:04 INFO - #01: COMPtr_Bloat_Smart_Test::TestBody() [xpcom/tests/gtest/TestCOMPtr.cpp:234]

So |COMPtr.Bloat_Smart| is crashing at:

> nsresult rv = CreateIBar( getter_AddRefs(barP) );

Where CreateIBar looks like:

> CreateIBar( void** result )
> {
>   IBar* barp = new IBar;
>   barp->AddRef();
>   *result = barp;
>   return NS_OK;
> }

The assertion is in:

>  void Assert_NoQueryNeeded()
>  {
>    if (mRawPtr) {
>      nsCOMPtr<T> query_result(do_QueryInterface(mRawPtr));
>      NS_ASSERTION(query_result.get() == mRawPtr, "QueryInterface needed");
>    }
>  }

Comment 9

[Eric Rahm [:erahm]]

Okay I can repro with the whole patchset applied which implies some misbehavior from having multiple file-scoped definitions of IFoo or IBar.

Comment 10

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/351981a14a43bb1902746687609d7aeaf1168c0a
Bug 1313470 - Part 0: Cleanup indentation. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/733ef60958c16161f51d2c345e6ac6c2fb8bedcb
Bug 1313470 - Part 1: Split out test functions. r=froydnj

https://hg.mozilla.org/integration/mozilla-inbound/rev/fa89c7fb96577c9eeb930279e30d9d6ab37baf7c
Bug 1313470 - Part 2: Convert TestCOMPtr to a gtest. r=froydnj

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/351981a14a43
https://hg.mozilla.org/mozilla-central/rev/733ef60958c1
https://hg.mozilla.org/mozilla-central/rev/fa89c7fb9657

Comment 12

[Eric Rahm [:erahm]]

For posterity: my fix was just to move the support code into a private namespace.