gfx-labeling Label runnables in gfx rendering.

RESOLVED FIXED in Firefox 55

Status

()

Core
Graphics
P1
normal
RESOLVED FIXED
10 months ago
7 months ago

People

(Reporter: vliu, Assigned: kechen)

Tracking

(Blocks: 1 bug)

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [gfx-noted][QDL][TDC-MVP][GFX])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

10 months ago
See https://wiki.mozilla.org/Quantum/DOM#Labeling for the story. Currently runnable categories in gfx rendering include sending ipc message between
  content and parent side [1].   

  ./gfx/layers/ipc/PImageBridge.ipdl
  ./gfx/layers/ipc/PLayerTransaction.ipdl
  ./gfx/layers/ipc/PTexture.ipdl
  ./gfx/layers/ipc/PWebRenderBridge.ipdl
  ./gfx/layers/ipc/PCompositorBridge.ipdl

  [1]; https://hg.mozilla.org/mozilla-central/annotate/e150eaff1f83e4e4a97d1e30c57d233859efe9cb/ipc/chromium/src/base/message_loop.cc#l286
(Reporter)

Updated

10 months ago
Blocks: 1341537

Updated

9 months ago
Whiteboard: [gfx-noted]
(Assignee)

Updated

9 months ago
Assignee: nobody → kechen
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 5

9 months ago
There are five ipc protocols under layers folder.
However, only three of them need to be labeled, and I will only do PLayerTransaction and PTexture in this bug.

1. The actors of PImageBridge are in main thread of compositor process and off-main of content process which do not need to be labled.
2. PWebRenderBridge is not completed yet, so I will leave this work in the future.
3. Messages in PCompositorBridge belong to different TabGroups, the messages should be labeled by messages, we will solve this problem in Bug 1333968.
(Assignee)

Comment 6

9 months ago
Hello Bevis, could you help me to review the patches for labeling part ? I will find someone to review graphics-related part later.

Comment 7

9 months ago
mozreview-review
Comment on attachment 8849389 [details]
Bug 1343754 - Add a function for replacing event target for actor and label PTexture;

https://reviewboard.mozilla.org/r/122166/#review124752

The way to set event target for an actor child is correct but there is still something we can refine.
Please see my comments below.

Thanks!

::: gfx/layers/ipc/CompositorBridgeChild.cpp:36
(Diff revision 2)
>  #include "nsIObserver.h"                // for nsIObserver
>  #include "nsISupportsImpl.h"            // for MOZ_COUNT_CTOR, etc
>  #include "nsTArray.h"                   // for nsTArray, nsTArray_Impl
>  #include "nsXULAppAPI.h"                // for XRE_GetIOMessageLoop, etc
>  #include "FrameLayerBuilder.h"
> +#include "mozilla/Dispatcher.h"

nit:
This include directive is not necessary because TabGroup.h includes Dispatcher.h

::: gfx/layers/ipc/CompositorBridgeChild.cpp:56
(Diff revision 2)
>  
>  using mozilla::layers::LayerTransactionChild;
>  using mozilla::dom::TabChildBase;
>  using mozilla::Unused;
>  using mozilla::gfx::GPUProcessManager;
> +using mozilla::dom::TabGroup;

You won't need these 2 using declaration because you won't explicitly use these 2 types as explained in the following comments.

::: gfx/layers/ipc/CompositorBridgeChild.cpp:891
(Diff revision 2)
> -                                          const uint64_t& aSerial)
> +                                          const uint64_t& aSerial,
> +                                          const uint64_t& aLayerId)
>  {
> -  return TextureClient::CreateIPDLActor();
> +  PTextureChild* textureChild = TextureClient::CreateIPDLActor();
> +
> +  RefPtr<Dispatcher> dispatcher;

You can remove this line as explained in next comment.

::: gfx/layers/ipc/CompositorBridgeChild.cpp:895
(Diff revision 2)
> +
> +  RefPtr<Dispatcher> dispatcher;
> +  TabChild* tabChild = TabChild::GetFrom(aLayerId);
> +
> +  // Do the DOM labeling.
> +  if (tabChild && aLayerId != 0) {

It's infallible to get a TabGroup from a TabChild and to get an EventTarget from a TabGroup, so you can simplify these labeling as followed:
```cpp
if (tabChild && aLayerId != 0) {
  SetEventTargetForActor(
    textureChild, 
    tabChild->TabGroup()->EventTargetFor(TaskCategory::Other));
  MOZ_ASSERT(textureChild->GetActorEventTarget());
}
```

::: gfx/layers/ipc/PCompositorBridge.ipdl:241
(Diff revision 2)
>    /**
>     * Sent when the child has finished CaptureAllPlugins.
>     */
>    async AllPluginsCaptured();
>  
> -  async PTexture(SurfaceDescriptor aSharedData, LayersBackend aBackend, TextureFlags aTextureFlags, uint64_t id, uint64_t aSerial);
> +  async PTexture(SurfaceDescriptor aSharedData, LayersBackend aBackend, TextureFlags aTextureFlags, uint64_t id, uint64_t aSerial, uint64_t aLayerId);

IMO, the aLayerId is not meaningful to the IPC protocol but is only used for setting EventTarget.

The Alternative way is to 
1. override PCompositorBridgeChild::SendPTextureConstructor in CompositorBridgeChild.
2. Call TextureClient::CreateIPDLActor() in this override function.

Then, you don't have to change anything in parent side.
Attachment #8849389 - Flags: review?(btseng)

Comment 8

9 months ago
mozreview-review
Comment on attachment 8849390 [details]
Bug 1343754 - Label PLayerTransaction;

https://reviewboard.mozilla.org/r/122168/#review124768

f=me after the following comments are addressed.

Thanks!

::: gfx/layers/ipc/CompositorBridgeChild.cpp:338
(Diff revision 2)
>  CompositorBridgeChild::AllocPLayerTransactionChild(const nsTArray<LayersBackend>& aBackendHints,
>                                                     const uint64_t& aId,
>                                                     TextureFactoryIdentifier*,
>                                                     bool*)
>  {
> +  RefPtr<Dispatcher> dispatcher;

Remove this line.

::: gfx/layers/ipc/CompositorBridgeChild.cpp:346
(Diff revision 2)
>    c->AddIPDLReference();
> +
> +  TabChild* tabChild = TabChild::GetFrom(c->GetId());
> +
> +  // Do the DOM Labeling.
> +  if (tabChild) {

Same as PTexture, we could simplify as follow:
```cpp
if (tabChild) {
  SetEventTargetForActor(
    c, tabChild->TabGroup()->EventTargetFor(TaskCategory::Other));  
}
```
Attachment #8849390 - Flags: review?(btseng)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 11

9 months ago
mozreview-review
Comment on attachment 8849389 [details]
Bug 1343754 - Add a function for replacing event target for actor and label PTexture;

https://reviewboard.mozilla.org/r/122166/#review124796

f=me, after the nit is addressed.
Thanks!

::: gfx/layers/ipc/CompositorBridgeChild.cpp:1043
(Diff revision 3)
> +  // Do the DOM labeling.
> +  if (tabChild && aLayerId != 0) {
> +    nsCOMPtr<nsIEventTarget> target =
> +      tabChild->TabGroup()->EventTargetFor(TaskCategory::Other);
> +    SetEventTargetForActor(textureChild, target);
> +  }

nit:
You can add this right after SetEventTargetForActor() to ensure that the EventTarget is set properly.
MOZ_ASSERT(textureChild->GetActorEventTarget());
Attachment #8849389 - Flags: review?(btseng)

Comment 12

9 months ago
mozreview-review
Comment on attachment 8849390 [details]
Bug 1343754 - Label PLayerTransaction;

https://reviewboard.mozilla.org/r/122168/#review124802

f=me, thanks!

::: gfx/layers/ipc/CompositorBridgeChild.cpp:344
(Diff revision 3)
> +
> +  // Do the DOM Labeling.
> +  if (tabChild) {
> +    nsCOMPtr<nsIEventTarget> target =
> +      tabChild->TabGroup()->EventTargetFor(TaskCategory::Other);
> +    SetEventTargetForActor(c, target);

nit:
MOZ_ASSERT(c->GetActorEventTarget());
Attachment #8849390 - Flags: review?(btseng)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 15

9 months ago
mozreview-review
Comment on attachment 8849389 [details]
Bug 1343754 - Add a function for replacing event target for actor and label PTexture;

https://reviewboard.mozilla.org/r/122166/#review125748
Attachment #8849389 - Flags: review?(btseng) → review+

Comment 16

9 months ago
mozreview-review
Comment on attachment 8849390 [details]
Bug 1343754 - Label PLayerTransaction;

https://reviewboard.mozilla.org/r/122168/#review125750
Attachment #8849390 - Flags: review?(btseng) → review+

Comment 17

9 months ago
mozreview-review
Comment on attachment 8849390 [details]
Bug 1343754 - Label PLayerTransaction;

https://reviewboard.mozilla.org/r/122168/#review125932

::: gfx/layers/ipc/CompositorBridgeChild.cpp:338
(Diff revision 4)
> +  TabChild* tabChild = TabChild::GetFrom(c->GetId());
> +
> +  // Do the DOM Labeling.
> +  if (tabChild) {
> +    nsCOMPtr<nsIEventTarget> target =

You can compress this a little to:

// Do the DOM labeling.
if (TabChild* tabChild = TabChild::GetFrom(aId)) {
  ...
Attachment #8849390 - Flags: review?(bugmail) → review+
Attachment #8849389 - Flags: review?(bugmail) → review?(nical.bugzilla)

Comment 18

9 months ago
mozreview-review
Comment on attachment 8849389 [details]
Bug 1343754 - Add a function for replacing event target for actor and label PTexture;

https://reviewboard.mozilla.org/r/122166/#review125934

::: gfx/layers/client/TextureClient.cpp:894
(Diff revision 4)
>    SurfaceDescriptor desc;
>    if (!ToSurfaceDescriptor(desc)) {
>      return false;
>    }
>  
> +  uint64_t layerId = 0;

This should be "layersId" instead of "layerId" - here and everywhere else in this patch

::: gfx/layers/client/TextureClient.cpp:896
(Diff revision 4)
>      return false;
>    }
>  
> +  uint64_t layerId = 0;
> +  if (XRE_IsContentProcess() && NS_IsMainThread()) {
> +    layerId = static_cast<ShadowLayerForwarder*>(aForwarder)

I don't think this is guaranteed to always be a ShadowLayerForwarder. In particular, with webrender enabled, this might be a WebRenderBridgeChild. I'm not 100% sure though - I think nical would be better to review this.

Comment 19

9 months ago
mozreview-review
Comment on attachment 8849389 [details]
Bug 1343754 - Add a function for replacing event target for actor and label PTexture;

https://reviewboard.mozilla.org/r/122166/#review125940

::: gfx/layers/client/TextureClient.cpp:896
(Diff revision 4)
>      return false;
>    }
>  
> +  uint64_t layerId = 0;
> +  if (XRE_IsContentProcess() && NS_IsMainThread()) {
> +    layerId = static_cast<ShadowLayerForwarder*>(aForwarder)

As Kats said, this can be a WebRenderBridgeChild, or even an ImageBridgeChild.

Comment 20

9 months ago
mozreview-review
Comment on attachment 8849389 [details]
Bug 1343754 - Add a function for replacing event target for actor and label PTexture;

https://reviewboard.mozilla.org/r/122166/#review126280

R- for now because we can't static_cast into ShadowLayerForwarder in InitIPDLActor. You can use a virtual upcast method like AsLayerForwarder (or add it if isn't there). Note that texture actors can move between tabs because we share the texture pool between all tabs of a given content process, so I assume that you need to be able to change the label of an existing PTexture actor when that happens. When a texture moves to another forwarder, it takes the branch "if (currentFwd != aForwarder) {" in InitIPDLActor so perhaps there is a bit of glue to add in there.
Also as kats said, please rename "layerId" into "layersId" or "layerForwarderId" because layerId is misleading in this context.
Attachment #8849389 - Flags: review?(nical.bugzilla) → review-
(Assignee)

Comment 21

9 months ago
Hello Nicolas,
After looking into the code, I found that the texture recycling mechanism is limited within the same forwarder since TextureClientRecycleAllocator is in CompositableClient[1] and currently I cannot find codes that reuse CompositableClient between tabs.
Therefore, we don't have to reset the eventTarget when doing the texture recycle work.

Please correct me if I miss something.

Thank you.

[1] https://dxr.mozilla.org/mozilla-central/rev/9577ddeaafd85554c2a855f385a87472a089d5c0/gfx/layers/client/CompositableClient.h#186
Flags: needinfo?(nical.bugzilla)
Unfortunately we have several (too many) ways to recycle textures. The one you are looking at is used mainly for video frames. The one we use for tiles is shared between tabs. See TextureClientPool.h/cpp, it uses TextureForwarder which is the CompositorBridgeChild.
Flags: needinfo?(nical.bugzilla)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Whiteboard: [gfx-noted] → [gfx-noted][QDL][TDC-MVP][GFX]

Comment 25

9 months ago
mozreview-review
Comment on attachment 8849389 [details]
Bug 1343754 - Add a function for replacing event target for actor and label PTexture;

https://reviewboard.mozilla.org/r/122166/#review127132

Looks good! With this patch, VR and video related texture actors are not labelled (I suppose this is intentional but it's worth keeping in mind).
Attachment #8849389 - Flags: review?(nical.bugzilla) → review+
(Assignee)

Comment 26

9 months ago
Hello Bill,

We got a problem when labeling the ipc PTexture in this bug.
As comment 20 mentioned, we will recycle the actor within the tabs without unregistering it, which means the owning tab of this protocol might be changed after it is created.

I've tried to change the TabGroup in the code but it ran into the assertion in [1].
Are there any ways to reset the event target without unregistering the actor ?


[1]https://dxr.mozilla.org/mozilla-central/source/ipc/glue/ProtocolUtils.cpp#861
Flags: needinfo?(wmccloskey)
No, there isn't any way to do that. It would be hard to know what to do about messages that are already in the message queue but haven't been dispatched yet. Is there a reason to recycle actors in this way? They're not very expensive to create.
Flags: needinfo?(wmccloskey)
(Assignee)

Comment 28

8 months ago
Hello Nicolas,
Based on comment 27, is it possible that an actor is recycled while there are still some messages in task queue that haven't been processed yet ?
Flags: needinfo?(nical.bugzilla)
(Assignee)

Comment 29

8 months ago
(In reply to Kevin Chen[:kechen] (UTC + 8) from comment #28)
> Hello Nicolas,
> Based on comment 27, is it possible that an actor is recycled while there
> are still some messages in task queue that haven't been processed yet ?

To be clear, the actor here means PTexture.
(Assignee)

Comment 30

8 months ago
Hello Sotaro,

Could you help me to take a look at comment 28?
Flags: needinfo?(nical.bugzilla) → needinfo?(sotaro.ikeda.g)
(In reply to Kevin Chen[:kechen] (UTC + 8) from comment #28)
> Hello Nicolas,
> Based on comment 27, is it possible that an actor is recycled while there
> are still some messages in task queue that haven't been processed yet ?

:kechen, how did you reproduce the problem? In which platform?

Updated

8 months ago
Flags: needinfo?(sotaro.ikeda.g) → needinfo?(kechen)
(Assignee)

Updated

8 months ago
Flags: needinfo?(kechen)
Priority: -- → P1
(In reply to Kevin Chen[:kechen] (UTC + 8) from comment #30)
> Hello Sotaro,
> 
> Could you help me to take a look at comment 28?

With the patch, a crash happened around TextureClientPool and TiledContentClient.
(Assignee)

Comment 33

8 months ago
Hello Sotaro,

The whole story is that when doing the ipc message labeling, we bind the actors with a TabGroup so the program will know the event target of incoming messages.
But in the PTexture case, the TabGroup of an actor can be changed while the it is recycled; however, gecko doesn't allow us to do such change, because it would be very hard to deal with the messages that have been in message queue.

My question is that, does the "PTexture actor is recycled while there are still some messages for this actor in message queue" thing will actually happened ?
If we can guarantee that the messages which belong to PTexture will be dispatched before the PTexture actor is recycled, maybe we could loose the restriction and change the TabGroup directly.
Flags: needinfo?(sotaro.ikeda.g)
With the patch crash was happened at "MOZ_RELEASE_ASSERT(aActor->Id() == kNullActorId || aActor->Id() == kFreedActorId)"

>  https://dxr.mozilla.org/mozilla-central/source/ipc/glue/ProtocolUtils.cpp#861

The comment say the following.

>  // We should only call this function on actors that haven't been used for IPC
>  // code yet. Otherwise we'll be posting stuff to the wrong event target before
>  // we're called.


But the function is called for aActor that is recycled.
:kechen, so, is it expected that the current patch cause the crash?
Flags: needinfo?(sotaro.ikeda.g) → needinfo?(kechen)
(In reply to Kevin Chen[:kechen] (UTC + 8) from comment #33)
> 
> My question is that, does the "PTexture actor is recycled while there are
> still some messages for this actor in message queue" thing will actually
> happened ?

Layer has several recyclings of TextureClients. TabGroup of an actor could be changed by TextureClients of TiledContentClient and TextureClients of ImageBridge(for video).

Recycling TextureClients of ImageBridge(for video) has guarantee that actor is not in message queue because of way of recycling. But in theory, TextureClients of ImageBridge(for video) could be used by multipe tab groups concurrently.

TextureClients of TiledContentClient has a different recycling mechanism than ImageBridge(for video). It tries to avoid it, but architectural guarantee is not clear enough. If it is broken by a bug, we do not have a clear way to detect it.
(Assignee)

Comment 37

8 months ago
(In reply to Sotaro Ikeda [:sotaro] from comment #35)
> :kechen, so, is it expected that the current patch cause the crash?

Yes, the patch should crash because this case is not covered by current mechanism.
Flags: needinfo?(kechen)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 40

8 months ago
Hello Bill,

About the PTexture actor recycling problem we've discussed in Comment 27.
Since we recycle the TextureClients frequently in the program, it might be not a good idea to recreate the actor every time.
Also, we can guarantee that there won't be any messages for PTextureChild in the task queue while we are changing the event target of the actor.

Therefore, when doing the texture recycling, I unregistered the actor first and reassigned the new event target for it [1].
Is it a proper way to solve this problem?

TL; DR
After the investigation, the only message that PTextureChild can receive on child side's main thread is __delete__[2].
And the __delete__ message can only be sent by parent process after the child side sends Destroy to parent.

In other words, the only time that we might get the PTexture's message in content side's task queue is after PTextureChild send the Destroy message. And we won't recycle a Texture which has been destroyed [3].

Therefore, it is guaranteed that there won't be any messages in the task queue while we are changing the event target of the actor.

[1] https://reviewboard.mozilla.org/r/122166/diff/6#index_header
[2] https://dxr.mozilla.org/mozilla-central/rev/f40e24f40b4c4556944c762d4764eace261297f5/gfx/layers/ipc/PTexture.ipdl#29
[3] https://dxr.mozilla.org/mozilla-central/rev/f40e24f40b4c4556944c762d4764eace261297f5/gfx/layers/client/TextureClient.cpp#866
Flags: needinfo?(wmccloskey)
(Assignee)

Updated

8 months ago
Depends on: 1350828

Comment 41

8 months ago
mozreview-review
Comment on attachment 8849389 [details]
Bug 1343754 - Add a function for replacing event target for actor and label PTexture;

https://reviewboard.mozilla.org/r/122166/#review133054

I don't think this will work.

::: gfx/layers/client/TextureClient.cpp:871
(Diff revision 6)
>  
>    if (mActor && !mActor->mDestroyed) {
>      CompositableForwarder* currentFwd = mActor->mCompositableForwarder;
>      TextureForwarder* currentTexFwd = mActor->mTextureForwarder;
>      if (currentFwd != aForwarder) {
> +      mActor->Unregister(mActor->Id());

Once you unregister an actor, it is removed from the actor map. I don't see where it would get added back to the map. That seems very broken.
Attachment #8849389 - Flags: review?(wmccloskey) → review-
I think we'll need to add a new SetEventTargetForActor method that specifically works even when the actor is already registered. Then we can call that method before SendDestroy.

However, it seems like we should just be able to eliminate this kind of actor the same way we've done for Layer and Compositable. I don't understand what purpose it serves. Would that be possible Matt?
Flags: needinfo?(wmccloskey) → needinfo?(matt.woodrow)
Note that we don't have time to deal with the fallout from any unexpected changes so I would suggest we stick wit the actor recycling unless we're 100% certain we don't need it.
Flags: needinfo?(nical.bugzilla)
(In reply to Bill McCloskey (:billm) from comment #42)
> I think we'll need to add a new SetEventTargetForActor method that
> specifically works even when the actor is already registered. Then we can
> call that method before SendDestroy.
> 
> However, it seems like we should just be able to eliminate this kind of
> actor the same way we've done for Layer and Compositable. I don't understand
> what purpose it serves. Would that be possible Matt?

David was looking into this, and iirc the conclusion was that it was possible, but not entirely trivial since there's a lot of complex code dealing with textures.

There wasn't an obvious huge advantage to doing this work (and non-zero risk), so it's been put aside for now.
Flags: needinfo?(matt.woodrow)
> the conclusion was that it was possible, but not entirely trivial since there's a lot of complex code dealing with textures.

I agree.
Flags: needinfo?(nical.bugzilla)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 48

8 months ago
mozreview-review
Comment on attachment 8849389 [details]
Bug 1343754 - Add a function for replacing event target for actor and label PTexture;

https://reviewboard.mozilla.org/r/122166/#review137498

Thanks. Sorry this took so long.

::: gfx/layers/client/TextureClient.cpp:12
(Diff revision 7)
>  #include <stdint.h>                     // for uint8_t, uint32_t, etc
>  #include "Layers.h"                     // for Layer, etc
>  #include "gfx2DGlue.h"
>  #include "gfxPlatform.h"                // for gfxPlatform
>  #include "mozilla/Atomics.h"
> +#include "mozilla/dom/TabGroup.h"

I don't think you should need this.

::: gfx/layers/ipc/CompositorBridgeChild.cpp:37
(Diff revision 7)
>  #include "nsISupportsImpl.h"            // for MOZ_COUNT_CTOR, etc
>  #include "nsTArray.h"                   // for nsTArray, nsTArray_Impl
>  #include "nsXULAppAPI.h"                // for XRE_GetIOMessageLoop, etc
>  #include "FrameLayerBuilder.h"
>  #include "mozilla/dom/TabChild.h"
> +#include "mozilla/dom/TabGroup.h"

Same here.

::: ipc/glue/ProtocolUtils.cpp:906
(Diff revision 7)
> +  // The EventTarget of a ToplevelProtocol shall never be set.
> +  MOZ_RELEASE_ASSERT(aActor != this);
> +
> +  int32_t id = aActor->Id();
> +  // The ID of the actor should have existed.
> +  MOZ_RELEASE_ASSERT(id);

Please instead assert it's != kNullActorId and != kFreedActorId.
Attachment #8849389 - Flags: review?(wmccloskey) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 53

7 months ago
The try run looks good.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c9dbae458de882f1d86f6b459887f979d58e56d
Keywords: checkin-needed

Comment 54

7 months ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/25f2e9e2068d
Add a function for replacing event target for actor and label PTexture; r=bevistseng,billm,nical
https://hg.mozilla.org/integration/autoland/rev/a5037670cb10
Label PLayerTransaction; r=bevistseng,kats
Keywords: checkin-needed
Backed out for crashing in some e10s tests on OSX 10.10 debug, e.g. in dom/workers/test/serviceworkers/test_fetch_integrity.html:

https://hg.mozilla.org/integration/autoland/rev/19b6ddc0671775aa85d5d200b56655834fda1f8e
https://hg.mozilla.org/integration/autoland/rev/a17e6caf485dd5277ed4354a5cf5b9047c7ad03d

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=a5037670cb10ca5d6cbc3b6c1f4d6f6a273932f9&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=96184860&repo=autoland
Flags: needinfo?(kechen)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 58

7 months ago
Already updated the patches, looks like I failed to upload them to the reviewboard.
I will push another try and mark checkin-needed again if everything is okay.
Flags: needinfo?(kechen)
(Assignee)

Updated

7 months ago
Keywords: checkin-needed

Comment 59

7 months ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0630447dd5b6
Add a function for replacing event target for actor and label PTexture; r=bevistseng,billm,nical
https://hg.mozilla.org/integration/autoland/rev/e92e9340219c
Label PLayerTransaction; r=bevistseng,kats
Keywords: checkin-needed

Comment 60

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0630447dd5b6
https://hg.mozilla.org/mozilla-central/rev/e92e9340219c
Status: NEW → RESOLVED
Last Resolved: 7 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.