Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Description • 9 years ago

Please see bug 1172796 comment 15. We need SourceSurfaces to be moved between threads because ImageBitmap stores SourfaceSurface and ImageBitmap can be transferred between threads.

My proposal is as following:
1. Let SourfaceSurface use thread-safe reference count.
2. Add a member to SourceSurface, this member indicate which thread is using this SourceSurface.
3. Add assertions to all member functions if owning thread and current thread mismatch.

How do you think, bas?

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 1 • 9 years ago

I don't think we should give SourceSurface a thread-safe reference count if it's only supposed to be owned by one thread at a time. That would give us a false sense of security.

Maybe a better approach would be to introduce an explicit API for transferring read-only SourceSurface data between threads. Perhaps some kind of ThreadSafeSurfaceSnapshot object you can create from a SourceSurface, transfer to another thread, and use to instantiate a SourceSurface, all without copying.

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 2 • 9 years ago

Bug 1215005 - Allow SourceSurface move between thread. r=roc

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 3 • 9 years ago

Comment on attachment 8678738 [details]
MozReview Request: Bug 1215005 - SourceSurface has thread-safe refcount now. r=bas

https://reviewboard.mozilla.org/r/23223/#review20813

This looks basically good.

For the next iteration of the patch, Bas and mattwoodrow should review it.

We'll also need followup patches for the other platforms.

::: gfx/2d/2D.h:321
(Diff revision 1)
> + * thread and call CreateSourceSurface to get a new SourfaceSurface in

SourceSurface

::: gfx/2d/2D.h:322
(Diff revision 1)
> + * that thread.

You can only call CreateSourceSurface once, right? Better document this here.

::: gfx/2d/2D.h:372
(Diff revision 1)
> +   * invalid or read-only depend on implementation.

I think we should try to be consistent here and make the SourceSurface always invalid after a transfer. We should assert if someone tries to use it for anything, and/or change its size to zero.

::: gfx/2d/SourceSurfaceCairo.h:28
(Diff revision 1)
> +};

This class doesn't need to be in the .h file, right?

::: gfx/2d/SourceSurfaceCairo.h:72
(Diff revision 1)
> +};

Likewise this class doesn't need to be in the .h file?

::: gfx/2d/SourceSurfaceRawData.h:128
(Diff revision 1)
> +  bool mOwnData;

This one too?

I think if mOwnData is false we shouldn't allow creation of a ThreadSafeSurfaceSnapshot. That seems very dangerous.

::: gfx/2d/SourceSurfaceRawData.h:222
(Diff revision 1)
> +  IntSize mSize;

Again, I don't think this needs to be in the .h file.

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 4 • 9 years ago

Comment on attachment 8678738 [details]
MozReview Request: Bug 1215005 - SourceSurface has thread-safe refcount now. r=bas

Bug 1215005 - Allow SourceSurface move between thread. r=roc r=bas r=mattwoodrow

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 5 • 9 years ago

Bug 1215005 - Add transfer support for SourceSurfaceCG. r=roc r=bas r=mattwoodrow

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 6 • 9 years ago

Bug 1215005 - Add transfer support for SourceSurfaceSkia. r=roc r=bas r=mattwoodrow

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 7 • 9 years ago

Comment on attachment 8678738 [details]
MozReview Request: Bug 1215005 - SourceSurface has thread-safe refcount now. r=bas

Bug 1215005 - Allow SourceSurface move between thread. r=roc r=bas r=mattwoodrow

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 8 • 9 years ago

Comment on attachment 8679890 [details]
MozReview Request: Bug 1215005 - Add transfer support for SourceSurfaceCG. r=roc r=bas r=mattwoodrow

Bug 1215005 - Add transfer support for SourceSurfaceCG. r=roc r=bas r=mattwoodrow

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 9 • 9 years ago

Comment on attachment 8679891 [details]
MozReview Request: Bug 1215005 - Add transfer support for SourceSurfaceSkia. r=roc r=bas r=mattwoodrow

Bug 1215005 - Add transfer support for SourceSurfaceSkia. r=roc r=bas r=mattwoodrow

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 10 • 9 years ago

Comment on attachment 8678738 [details]
MozReview Request: Bug 1215005 - SourceSurface has thread-safe refcount now. r=bas

https://reviewboard.mozilla.org/r/23223/#review21005

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 11 • 9 years ago

Comment on attachment 8679890 [details]
MozReview Request: Bug 1215005 - Add transfer support for SourceSurfaceCG. r=roc r=bas r=mattwoodrow

https://reviewboard.mozilla.org/r/23543/#review21007

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 12 • 9 years ago

Comment on attachment 8679891 [details]
MozReview Request: Bug 1215005 - Add transfer support for SourceSurfaceSkia. r=roc r=bas r=mattwoodrow

https://reviewboard.mozilla.org/r/23545/#review21009

	Bas Schouten (:bas.schouten)
	Comment 13 • 9 years ago

Comment on attachment 8678738 [details]
MozReview Request: Bug 1215005 - SourceSurface has thread-safe refcount now. r=bas

https://reviewboard.mozilla.org/r/23223/#review21019

::: gfx/2d/2D.h:13
(Diff revision 2)
> +#include "nsISupportsImpl.h"

nsISupportsImpl is not in MFBT and therefore cannot be used inside Moz2D.

This is actually nothing like the intent we actually have with SourceSurfaces. Will comment further on the bug.

	Bas Schouten (:bas.schouten)
	Comment 14 • 9 years ago

Actually, I'm very sorry that I didn't answer the original needinfo in time here.

The intent here is that SourceSurfaces are inherently threadsafe (this is required for numerous reasons, some of which lying in other multi-threaded efforts that are currently occurring).

Essentially there's for we should make the reference counting threadsafe, and we need to add support in the Map and Unmap functions of DataSourceSurface to synchronize access to SourceSurfaces from multiple threads.

There may still be some value in adding the ability to take a (copy-onwrite or readonly, i.e. one which will simply refuse write maps) Snapshot of a SourceSurface. But I don't think that usecase exists at this point, if I'm wrong though I'd love to hear about it.

Again, I apologize for not seeing this bug and getting involved earlier.

	Bas Schouten (:bas.schouten)
	Comment 15 • 9 years ago

Comment on attachment 8679890 [details]
MozReview Request: Bug 1215005 - Add transfer support for SourceSurfaceCG. r=roc r=bas r=mattwoodrow

https://reviewboard.mozilla.org/r/23543/#review21021

	Bas Schouten (:bas.schouten)
	Comment 16 • 9 years ago

Comment on attachment 8679891 [details]
MozReview Request: Bug 1215005 - Add transfer support for SourceSurfaceSkia. r=roc r=bas r=mattwoodrow

https://reviewboard.mozilla.org/r/23545/#review21023

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 17 • 9 years ago

(In reply to Bas Schouten (:bas.schouten) from comment #14)
> Actually, I'm very sorry that I didn't answer the original needinfo in time
> here.
> 
> The intent here is that SourceSurfaces are inherently threadsafe (this is
> required for numerous reasons, some of which lying in other multi-threaded
> efforts that are currently occurring).
This is my proposal in comment 0, but roc says in comment 1 there would get more problem is we let SourceSurface threadsafe. So I abandoned this proposal.

> 
> Essentially there's for we should make the reference counting threadsafe,
> and we need to add support in the Map and Unmap functions of
> DataSourceSurface to synchronize access to SourceSurfaces from multiple
> threads.
> 
> There may still be some value in adding the ability to take a (copy-onwrite
> or readonly, i.e. one which will simply refuse write maps) Snapshot of a
> SourceSurface. But I don't think that usecase exists at this point, if I'm
> wrong though I'd love to hear about it.
So the alternative way is create a snapshot of SourceSurface and let this snapshot threadsafe.

The main usecase for this snapshot is OffscreenCanvas which would be implemented in bug 1172796. In the OffscreenCanvas, we can create ImageBitmap in worker by calling OffscreenCanvas.transferToImageBitmap(). And then we can transfer this ImageBitmap to arbitary threads. The underlying buffer of ImageBitmap is actually a SourceSurface. So transfer a ImageBitmap means transfering a SourceSurface. Thus we need this snapshot mechanism to transfer ownership to arbitary thread when we transfer ImageBitmap. How do you think, bas?
> 
> Again, I apologize for not seeing this bug and getting involved earlier.

	Matt Woodrow (:mattwoodrow)
	Comment 18 • 9 years ago

Comment on attachment 8678738 [details]
MozReview Request: Bug 1215005 - SourceSurface has thread-safe refcount now. r=bas

https://reviewboard.mozilla.org/r/23223/#review21331

I'm not a huge fan of having functions that neuter objects, where we then have to rely on the caller to stop using them.

I'll wait until we have agreement from Bas on which high level approach we want to take before reviewing this too much.

::: gfx/2d/SourceSurfaceCairo.cpp:70
(Diff revision 2)
> +    cairo_surface_destroy(mSurface);

Set mSurface to nullptr too here, and MOZ_ASSERT that mSurface is !nullptr at the start.
We want to make sure nobody calls this function twice.

::: gfx/2d/SourceSurfaceCairo.cpp:145
(Diff revision 2)
> +  mSize.height = 0;

Can we drop the reference to mSurface and clear the pointer too?

Just clearing the width/height won't make this entirely unusable (GetNativeSurface?), which is what we want I think. Overriding SourceSurface::IsValid would be a good idea too.

	Bas Schouten (:bas.schouten)
	Comment 19 • 9 years ago

(In reply to Morris Tseng [:mtseng] from comment #17)
> (In reply to Bas Schouten (:bas.schouten) from comment #14)
> > Actually, I'm very sorry that I didn't answer the original needinfo in time
> > here.
> > 
> > The intent here is that SourceSurfaces are inherently threadsafe (this is
> > required for numerous reasons, some of which lying in other multi-threaded
> > efforts that are currently occurring).
> This is my proposal in comment 0, but roc says in comment 1 there would get
> more problem is we let SourceSurface threadsafe. So I abandoned this
> proposal.

Well, roc's assumption was SourceSurfaces should only be owned by one thread at a time. If that was true, then he would be absolutely right, it's a bad idea to just make the refcount threadsafe.

However that assumption isn't true, some SourceSurfaces are already threadsafe, some still need to be made as such.

> > There may still be some value in adding the ability to take a (copy-onwrite
> > or readonly, i.e. one which will simply refuse write maps) Snapshot of a
> > SourceSurface. But I don't think that usecase exists at this point, if I'm
> > wrong though I'd love to hear about it.
> So the alternative way is create a snapshot of SourceSurface and let this
> snapshot threadsafe.
> 
> The main usecase for this snapshot is OffscreenCanvas which would be
> implemented in bug 1172796. In the OffscreenCanvas, we can create
> ImageBitmap in worker by calling OffscreenCanvas.transferToImageBitmap().
> And then we can transfer this ImageBitmap to arbitary threads. The
> underlying buffer of ImageBitmap is actually a SourceSurface. So transfer a
> ImageBitmap means transfering a SourceSurface. Thus we need this snapshot
> mechanism to transfer ownership to arbitary thread when we transfer
> ImageBitmap. How do you think, bas?

Well, this depends a lot on what the eventual intention of OffscreenCanvas is and what performance characteristics are desirable. If the intent is to frequently update the content of the canvas and be able to composite it efficiently, potentially with new content every frame. A hidden double buffering implementation such as offered by the regular Canvas PersistentBufferProvider would be desirable, this would also provide snapshot capabilities and integrate easily with current canvas code. Having said that currently the PersistentBufferProvider only works on the main thread so that would have to be adapted a little, it's perfectly possible to make it work on other threads though, there's probably value in that!

If the intention is to have infrequent modifications and only occasional presentation or readback, that's a different story. If the main intent would be readback or accessing image data directly, it's possible to simply to get a DataSourceSurface. DataSourceSurfaces are threadsafe for reading, and in the near future should implement a copy-on-write mechanism for writing, if writing is allowed for that particular SourceSurface. If you want efficient presentation from the screen I would recommend some form of integrating with the current CanvasClient rather than the ImageClient.

If you really wanted to go the SourceSurface route with a form of ImageClient then the story is a little more complicated. Some SourceSurfaces are already perfectly threadsafe. Others are not, it would depend on what platforms/backends we'd be willing to do the work to make them threadsafe vs. where we'd need them.

Now having said all that, a much bigger problem I foresee is that currently most of the DrawTarget backends aren't fully threadsafe, i.e. most, if not all of our DTs can -not- be safely used off the main thread. It will work most of the time, and race conditions are 'unlikely' for most of our DrawTargets, but they're certainly there. This bug doesn't seem like the right place to discuss that though.

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 20 • 9 years ago

ImageBitmap is immutable, so we don't have modification usecases. And ImageBitmap cannot read by multiple threads at the same time (It passes to another thread by copy or transfer, so only one thread can use ImageBitmap at the same time). The issue I encounter is more like a lifetime issue. Consider following example.

1. Create a ImageBitmap which contains a SourceSurface in worker thread.
2. Send it to main-thread.
3. Shutdown the worker thread.

The problem is in step2, we don't have threadsafe refcount for SourceSurface which means SourceSurface should be destroyed in its creation thread. So in step 3 we should destroy this SourceSurface or we'll leak that SourceSurface. In the mean time, main thread may access the ImageBitmap as well and it might access a invalid ImageBitmap if we destroy it when we shutdown the worker thread.

Roc suggests me use a snapshot to describe ownership transferring explicitly so that we know which thread own this SourceSurface and we can easily solve this lifetime problem. But after you explanation, I have a another idea. I think since DataSourceSurface is threadsafe for reading which looks fit my requirement. Can we let DataSourceSurface is threadsafe for refcounting? So that I can use refcount to control lifetime of SourceSurface between threads. Although I think threadsafe recount is overkill because in my use case SourceSurface is owned by only one thread.

	Bas Schouten (:bas.schouten)
	Comment 21 • 9 years ago

(In reply to Morris Tseng [:mtseng] from comment #20)
> ImageBitmap is immutable, so we don't have modification usecases. And
> ImageBitmap cannot read by multiple threads at the same time (It passes to
> another thread by copy or transfer, so only one thread can use ImageBitmap
> at the same time). The issue I encounter is more like a lifetime issue.
> Consider following example.
> 
> 1. Create a ImageBitmap which contains a SourceSurface in worker thread.
> 2. Send it to main-thread.
> 3. Shutdown the worker thread.
> 
> The problem is in step2, we don't have threadsafe refcount for SourceSurface
> which means SourceSurface should be destroyed in its creation thread. So in
> step 3 we should destroy this SourceSurface or we'll leak that
> SourceSurface. In the mean time, main thread may access the ImageBitmap as
> well and it might access a invalid ImageBitmap if we destroy it when we
> shutdown the worker thread.
> 
> Roc suggests me use a snapshot to describe ownership transferring explicitly
> so that we know which thread own this SourceSurface and we can easily solve
> this lifetime problem. But after you explanation, I have a another idea. I
> think since DataSourceSurface is threadsafe for reading which looks fit my
> requirement. Can we let DataSourceSurface is threadsafe for refcounting? So
> that I can use refcount to control lifetime of SourceSurface between
> threads. Although I think threadsafe recount is overkill because in my use
> case SourceSurface is owned by only one thread.

Yes, we can do that, a lot of questions in my previous comment went unanswered though. Transferring the contents of an offscreen canvas through an immutable bitmap seems very inefficient. Why are be choosing this approach and what performance characteristics are we attempting to achieve?

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 22 • 9 years ago

OffscreenCanvas have 2 configurations. If we want fast composition, we can call canvas.transferControlToOffscreen() to get a OffscreenCanvas and transfer this OffscreenCanvas to worker. So this OffscreenCanvas is bound to a canvas element which will be shown on the screen. Thus, any draw operation on worker will present to compositor asap through our internal surface.

Another one is we can create OffscreenCanvas on worker directly. That is, this OffscreenCanvas doesn't associate to any canvas element. In this case, if main-thread want the result of OffscreenCanvas, worker could call OffscreenCanvas.transferImageBitmap() to get a ImageBitmap and then transfer this ImageBitmap to main-thread. So the intention here doesn't restrict for composition. It may become a source of another canvas or save to file depend on the application. This configuration is useful for image processing which no need to display result on the screen. In this configuration, we might not use it for composition. But we still want the number of copies as few as possible during the transferring process.

	Bas Schouten (:bas.schouten)
	Comment 23 • 9 years ago

(In reply to Morris Tseng [:mtseng] from comment #22)
> OffscreenCanvas have 2 configurations. If we want fast composition, we can
> call canvas.transferControlToOffscreen() to get a OffscreenCanvas and
> transfer this OffscreenCanvas to worker. So this OffscreenCanvas is bound to
> a canvas element which will be shown on the screen. Thus, any draw operation
> on worker will present to compositor asap through our internal surface.

How do we plan to do this presentation in a fast way at this point? And how do we get around the problem that DrawTargets cannot be used off the main thread?

> Another one is we can create OffscreenCanvas on worker directly. That is,
> this OffscreenCanvas doesn't associate to any canvas element. In this case,
> if main-thread want the result of OffscreenCanvas, worker could call
> OffscreenCanvas.transferImageBitmap() to get a ImageBitmap and then transfer
> this ImageBitmap to main-thread. So the intention here doesn't restrict for
> composition. It may become a source of another canvas or save to file depend
> on the application. This configuration is useful for image processing which
> no need to display result on the screen. In this configuration, we might not
> use it for composition. But we still want the number of copies as few as
> possible during the transferring process.

Hrm, so the W3C spec here guarantees that the ImageBitmap is always owned specifically by a certain thread?

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 24 • 9 years ago

(In reply to Bas Schouten (:bas.schouten) from comment #23)
> (In reply to Morris Tseng [:mtseng] from comment #22)
> How do we plan to do this presentation in a fast way at this point?
I already did it by updating CanvasClient through ImageBridge. Please check [1].

[1]: https://dxr.mozilla.org/mozilla-central/source/dom/canvas/OffscreenCanvas.cpp#186

> And how do we get around the problem that DrawTargets cannot be used off the main
> thread?
Right now OffscreenCanvas only supports WebGL. Bug 801176 will add 2d canvas support for OffscreenCanvas. So we don't need deal with DrawTarget on the off main thread now.
> 
> Hrm, so the W3C spec here guarantees that the ImageBitmap is always owned
> specifically by a certain thread?
Because there are 2 ways to pass ImageBitmap to another thread. First is structured clone which means copy a new ImageBitmap in another thread. Second is through transferring. Transferring also transfer ownership. So in those 2 cases, ImageBitmap is always owned by certain thread.

	Bas Schouten (:bas.schouten)
	Comment 25 • 9 years ago

(In reply to Morris Tseng [:mtseng] from comment #24)
> (In reply to Bas Schouten (:bas.schouten) from comment #23)
> > (In reply to Morris Tseng [:mtseng] from comment #22)
> > How do we plan to do this presentation in a fast way at this point?
> I already did it by updating CanvasClient through ImageBridge. Please check
> [1].
> 
> [1]:
> https://dxr.mozilla.org/mozilla-central/source/dom/canvas/OffscreenCanvas.
> cpp#186

It's a shame we have to copy every frame rather than having a double buffering mechanism, but oh well. I also wonder if this will properly support the OMTC case on windows from a quick glance, where we can use DXGI surfaces to share hardware accelerated surfaces between processes. Do you know?

> > And how do we get around the problem that DrawTargets cannot be used off the main
> > thread?
> Right now OffscreenCanvas only supports WebGL. Bug 801176 will add 2d canvas
> support for OffscreenCanvas. So we don't need deal with DrawTarget on the
> off main thread now.

Okay!

> > Hrm, so the W3C spec here guarantees that the ImageBitmap is always owned
> > specifically by a certain thread?
> Because there are 2 ways to pass ImageBitmap to another thread. First is
> structured clone which means copy a new ImageBitmap in another thread.
> Second is through transferring. Transferring also transfer ownership. So in
> those 2 cases, ImageBitmap is always owned by certain thread.

Yes, in that case if you just make sure the SourceSurfaces we need are threadsafe (if that's just DataSourceSurface that should be very easy), we can just make the refcount threadsafe and work with that.

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 26 • 9 years ago

(In reply to Bas Schouten (:bas.schouten) from comment #25)
> 
> It's a shame we have to copy every frame rather than having a double
> buffering mechanism, but oh well. I also wonder if this will properly
> support the OMTC case on windows from a quick glance, where we can use DXGI
> surfaces to share hardware accelerated surfaces between processes. Do you
> know?
In OffscreenCanvas for webgl case, we have double buffering like mechanism and it shared surface through DXGI as well. All mechanism in worker webgl just the same as normal webgl. So normal webgl has double buffering like mechanism and shared surface through DXGI, worker webgl does the same thing.
> 
> Yes, in that case if you just make sure the SourceSurfaces we need are
> threadsafe (if that's just DataSourceSurface that should be very easy), we
> can just make the refcount threadsafe and work with that.
Ok! But I have a question about this. Since SourceSurface inherits RefCounted, SourceSurface is not thread-safe refcounted. But DataSourceSurface inherits SourceSurface, how I change DataSourceSurface to thread-safe refcounted but not modified SourceSurface?

	Bas Schouten (:bas.schouten)
	Comment 27 • 9 years ago

(In reply to Morris Tseng [:mtseng] from comment #26)
> (In reply to Bas Schouten (:bas.schouten) from comment #25)
> > 
> > Yes, in that case if you just make sure the SourceSurfaces we need are
> > threadsafe (if that's just DataSourceSurface that should be very easy), we
> > can just make the refcount threadsafe and work with that.
> Ok! But I have a question about this. Since SourceSurface inherits
> RefCounted, SourceSurface is not thread-safe refcounted. But
> DataSourceSurface inherits SourceSurface, how I change DataSourceSurface to
> thread-safe refcounted but not modified SourceSurface?

Just making them all use threadsafe refcounting is fine. Right now I don't think it asserts when used on different threads anyway. Make sure you add to the documentation of SourceSurface though that not all of them can be used on random threads at the moment. (This should be fixed, but we need to make sure people realize that that's not yet the case)

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 28 • 9 years ago

Comment on attachment 8678738 [details]
MozReview Request: Bug 1215005 - SourceSurface has thread-safe refcount now. r=bas

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23223/diff/2-3/

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 29 • 9 years ago

Comment on attachment 8678738 [details]
MozReview Request: Bug 1215005 - SourceSurface has thread-safe refcount now. r=bas

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23223/diff/3-4/

	Bas Schouten (:bas.schouten)
	Comment 30 • 9 years ago

Comment on attachment 8678738 [details]
MozReview Request: Bug 1215005 - SourceSurface has thread-safe refcount now. r=bas

https://reviewboard.mozilla.org/r/23223/#review22313

::: gfx/2d/2D.h:16
(Diff revision 3)
> +#include "nsISupportsImpl.h" // For NS_INLINE_DECL_THREADSAFE_REFCOUNTING

You can't use this in Moz2D code.

::: gfx/2d/2D.h:328
(Diff revision 3)
> -class SourceSurface : public RefCounted<SourceSurface>
> +class SourceSurface

Just switch this to the AtomicRefCounted template class, that should be all you need.

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 31 • 9 years ago

Comment on attachment 8678738 [details]
MozReview Request: Bug 1215005 - SourceSurface has thread-safe refcount now. r=bas

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23223/diff/3-4/

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 32 • 9 years ago

I got some problem when using review board in this bug. Switch back to ordinary patch.

	Bas Schouten (:bas.schouten)
	Comment 33 • 9 years ago

Comment on attachment 8678738 [details]
MozReview Request: Bug 1215005 - SourceSurface has thread-safe refcount now. r=bas

https://reviewboard.mozilla.org/r/23223/#review23787

	Morris Tseng [:mtseng] [:Morris] (Inactive) Assignee
	Comment 34 • 9 years ago

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a24c8cab934c

	Pulsebot
	Comment 35 • 9 years ago

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

	Carsten Book [:Tomcat]
	Comment 36 • 9 years ago
bugherder

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