Closed Bug 1302641 Opened 7 years ago Closed 7 years ago

dom/media/webrtc/MediaEngineGonkVideoSource.cpp:417:89: error: use of deleted function 'RefPtr<T>::operator T*() const && [with T = mozilla::layers::ImageBridgeChild]'

Categories

(Firefox OS Graveyard :: Gaia::Build, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::Build
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox51 fixed)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox51 --- fixed

People

(Reporter: Vagrantin, Assigned: gerard-majax)

References

Details

Attachments

(2 files, 5 obsolete files)

Update ImageBridgeChild::GetSingleton() use after 1298938
4.29 KB, patch
gerard-majax
: review+
Details | Diff | Splinter Review
Update ImageBridgeChild::GetSingleton() use after 1298938
9.08 KB, patch
gerard-majax
: review+
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36
./shinano-common/extract-files.sh: line 68: adb: command not found
/home/worker/workspace/gecko/dom/media/webrtc/MediaEngineGonkVideoSource.cpp:417:89: error: use of deleted function 'RefPtr<T>::operator T*() const && [with T = mozilla::layers::ImageBridgeChild]'
make[6]: *** [Unified_cpp_dom_media_webrtc0.o] Error 1
make[5]: *** [dom/media/webrtc/target] Error 2
make[5]: *** Waiting for unfinished jobs....
make[4]: *** [compile] Error 2
make[3]: *** [default] Error 2
make[2]: *** [realbuild] Error 2
make[1]: *** [build] Error 2
make: *** [out/target/product/aries/obj/DATA/gecko_intermediates/gecko] Error 2
Return code: 2
failed to build
Running post_fatal callback...
Exiting 2
I believe this is a result of changes in https://bugzilla.mozilla.org/show_bug.cgi?id=1298938
I don't think the adb error is significant.  See for instance this task: https://tools.taskcluster.net/task-inspector/#A__IqIl_QN-t1Q6xr2HNyw/0.  The adb error is present there too, but does not affect the success of the build.
Blocks: 1245091
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-tier=1&filter-tier=2&filter-tier=3&filter-searchStr=b2g&fromchange=16c9625e4fa1c30aee3acb99912bb220b9b7e7d4&selectedJob=35786722
Depends on: 1298938
Summary: JOB failed : B2G Device Image opt Aries Device Image b2g-device-aries-eng/opt Aries(Be) → dom/media/webrtc/MediaEngineGonkVideoSource.cpp:417:89: error: use of deleted function 'RefPtr<T>::operator T*() const && [with T = mozilla::layers::ImageBridgeChild]'
I might have a fix ...
Comment on attachment 8791083 [details] [diff] [review]
Update ImageBridgeChild::GetSingleton() use after 1298938

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

::: dom/media/webrtc/MediaEngineGonkVideoSource.cpp
@@ +413,5 @@
>      // to explicitly remove this--destroying the CameraControl object
>      // in DeallocImpl() will do that for us.
>      mCameraControl->AddListener(this);
>      mTextureClientAllocator =
> +      new layers::TextureClientRecycleAllocator(layers::ImageBridgeChild::GetSingleton().get());

please change this to:

RefPtr<ImageBridgeChild> bridge = layers:::ImageBridgeChild::GetSignleton();
mTextureClientAllocator = new layers::TextureClientRecycleAllocator(bridge);

The idea is to make sure the RefPtr holds the ImageBridge refcount until at least after the allocator's constructor so that it doesn't get racily deleted in another thread. As far as I understand your code is already correct but the C++ standard's wording about the lifetime of a temporary value is a bit hard to read and not well known, so making it explicit will avoid someone puzzle about the correctness of this code in the future.
Attachment #8791083 - Flags: review?(nical.bugzilla) → review+
(In reply to Nicolas Silva [:nical] from comment #7)
> Comment on attachment 8791083 [details] [diff] [review]
> Update ImageBridgeChild::GetSingleton() use after 1298938
> 
> Review of attachment 8791083 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/media/webrtc/MediaEngineGonkVideoSource.cpp
> @@ +413,5 @@
> >      // to explicitly remove this--destroying the CameraControl object
> >      // in DeallocImpl() will do that for us.
> >      mCameraControl->AddListener(this);
> >      mTextureClientAllocator =
> > +      new layers::TextureClientRecycleAllocator(layers::ImageBridgeChild::GetSingleton().get());
> 
> please change this to:
> 
> RefPtr<ImageBridgeChild> bridge = layers:::ImageBridgeChild::GetSignleton();
> mTextureClientAllocator = new layers::TextureClientRecycleAllocator(bridge);
> 
> The idea is to make sure the RefPtr holds the ImageBridge refcount until at
> least after the allocator's constructor so that it doesn't get racily
> deleted in another thread. As far as I understand your code is already
> correct but the C++ standard's wording about the lifetime of a temporary
> value is a bit hard to read and not well known, so making it explicit will
> avoid someone puzzle about the correctness of this code in the future.

Thanks!

Meanwhle, the try revealed other places:
> /home/worker/workspace/gecko/dom/media/platforms/gonk/GonkVideoDecoderManager.cpp:403:90: error: use of deleted function 'RefPtr<T>::operator T*() const && 
> /home/worker/workspace/gecko/dom/media/platforms/omx/GonkOmxPlatformLayer.cpp:164:82: error: use of deleted function 'RefPtr<T>::operator T*() const && [with T = mozilla::layers::ImageBridgeChild]'
> /home/worker/workspace/gecko/dom/media/platforms/omx/GonkOmxPlatformLayer.cpp:172:85: error: use of deleted function 'RefPtr<T>::operator T*() const && [with T = mozilla::layers::ImageBridgeChild]'
I fixed the new problems using the same changes you suggested, they seemed to make sense there too, is that right?
In https://hg.mozilla.org/mozilla-central/rev/2d2b7d0a949a they use .get() for temp variables :)
Comment on attachment 8791137 [details] [diff] [review]
Update ImageBridgeChild::GetSingleton() use after 1298938

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

Thanks!
Attachment #8791137 - Flags: review?(nical.bugzilla) → review+

        Attachment #8791196 -
        Attachment is obsolete: true

        Attachment #8791218 -
        Flags: review+

    
    

    
  
Pushed by alissy@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a5cf829f56e
Update ImageBridgeChild::GetSingleton() use after 1298938 r=nical
Assignee: nobody → lissyx+mozillians
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/0a5cf829f56e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago

          status-firefox51:
          --- → fixed
Resolution: --- → FIXED

    
    

    
  
This issue is set as resolved by i'm seeing still build failling you'll find time and date because i don't know if bugzilla and treeherder have the same timezone (PDT for bugzilla)

Mozilla-Central,

Job  (sig)  :  B2G Device Image opt (and debug) Aries Device Image b2g-device-aries-eng/opt Aries(Be)
Machine: i-058edc65a9c9d8fc7
Build: - b2g-device-image -
Job name: b2g-device-aries-eng/opt


Requested: Thu Sep 15, 19:00:01
Started: Thu Sep 15, 19:00:04
Ended: Thu Sep 15, 19:37:23



./shinano-common/extract-files.sh: line 68: adb: command not found
/home/worker/workspace/gecko/dom/media/webrtc/MediaEngineGonkVideoSource.cpp:417:89: error: use of deleted function 'RefPtr<T>::operator T*() const && [with T = mozilla::layers::ImageBridgeChild]'
make[6]: *** [Unified_cpp_dom_media_webrtc0.o] Error 1
make[5]: *** [dom/media/webrtc/target] Error 2
make[5]: *** Waiting for unfinished jobs....
make[4]: *** [compile] Error 2
make[3]: *** [default] Error 2
make[2]: *** [realbuild] Error 2
make[1]: *** [build] Error 2
make: *** [out/target/product/aries/obj/DATA/gecko_intermediates/gecko] Error 2
Return code: 2
failed to build
Running post_fatal callback...
Exiting 



Matth
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

    
    

    
  
(In reply to m.ducorps from comment #21)
> This issue is set as resolved by i'm seeing still build failling you'll find
> time and date because i don't know if bugzilla and treeherder have the same
> timezone (PDT for bugzilla)
> 
> Mozilla-Central,
> 
> Job  (sig)  :  B2G Device Image opt (and debug) Aries Device Image
> b2g-device-aries-eng/opt Aries(Be)
> Machine: i-058edc65a9c9d8fc7
> Build: - b2g-device-image -
> Job name: b2g-device-aries-eng/opt
> 
> 
> Requested: Thu Sep 15, 19:00:01
> Started: Thu Sep 15, 19:00:04
> Ended: Thu Sep 15, 19:37:23
> 
> 
> 
> ./shinano-common/extract-files.sh: line 68: adb: command not found
> /home/worker/workspace/gecko/dom/media/webrtc/MediaEngineGonkVideoSource.cpp:
> 417:89: error: use of deleted function 'RefPtr<T>::operator T*() const &&
> [with T = mozilla::layers::ImageBridgeChild]'
> make[6]: *** [Unified_cpp_dom_media_webrtc0.o] Error 1
> make[5]: *** [dom/media/webrtc/target] Error 2
> make[5]: *** Waiting for unfinished jobs....
> make[4]: *** [compile] Error 2
> make[3]: *** [default] Error 2
> make[2]: *** [realbuild] Error 2
> make[1]: *** [build] Error 2
> make: *** [out/target/product/aries/obj/DATA/gecko_intermediates/gecko]
> Error 2
> Return code: 2
> failed to build
> Running post_fatal callback...
> Exiting 
> 
> 
> 
> Matth

Please give a link to the failure. The patch was not ready at the time of the merge, so obviously the failure went from mozilla-inbound to mozilla-central, but on the next merge, the issue was fixed ...

I just checked, m-c is still good, so I don't understand ...
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED

          You need to log in
          before you can comment on or make changes to this bug.