1335335 - Tab move between different windows does not work when e10s is enabled.

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Sotaro Ikeda [:sotaro]]

Tab move between different windows does not work yet.

Comment 1

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Add LayerManager::GetCompositorBridgeChild()

Comment 2

[Sotaro Ikeda [:sotaro]]

Attachment: wip patch

Comment 3

[Sotaro Ikeda [:sotaro]]

The followings needs to be addressed.
- WebRenderCommands are not kept in WebRenderBridgeParent.
    Client side needs to resend all commands in this case.
- ImageKey is local to each window.
   Each window has different webrender instance.

Comment 6

[Sotaro Ikeda [:sotaro]]

Attachment: wip patch

Comment 7

[Sotaro Ikeda [:sotaro]]

Attachment: wip patch

Comment 8

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Tab move between different windows when e10s is enabled

Comment 9

[Sotaro Ikeda [:sotaro]]

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

Comment 10

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Tab move between different windows when e10s is enabled

Fixed crash during tab move.

Comment 11

[Sotaro Ikeda [:sotaro]]

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

Comment 12

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Tab move between different windows when e10s is enabled

Comment 13

[Sotaro Ikeda [:sotaro]]

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

Comment 14

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Tab move between different windows when e10s is enabled

Comment 15

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Tab move between different windows when e10s is enabled

Comment 16

[Sotaro Ikeda [:sotaro]]

attachment 8880321 [details] [diff] [review] still has one problem. On windows, sometimes moved tabs were not re-rendered.

It was cased by un-handled obsoleted DPEnd message.

Obsoleted HandleDPEnd() was called after UpdateWebRender(). The obsoleted message was just dropped. Then client side just waited a transaction complete of the obsoleted message.

Comment 17

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 - Tab move between different windows when e10s is enabled

Comment 18

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 - Tab move between different windows when e10s is enabled

Update a comment.

Comment 19

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 - Manually call DidComposite() when transaction was dropped because of obsoleted command

Comment 20

[Sotaro Ikeda [:sotaro]]

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

Comment 21

[Nicolas Silva [:nical]]

Comment on attachment 8880726 [details] [diff] [review]
patch part 1 - Tab move between different windows when e10s is enabled

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

Please write a big comment explaining why it is hard to avoid tracking old and new ids. I am hopeful that some time in the future we can refactor the layers code so that we don't have to go through this complication.

Comment 22

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 - Tab move between different windows when e10s is enabled

Apply the comment.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac18f9244933
Tab move between different windows does not work when e10s is enabled r=nical
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5dbfdc1892e
Manually call DidComposite() when transaction was dropped because of obsoleted command r=nical

Comment 24

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for build bustage: version control conflict in maker file at WebRenderBridgeParent.h:235:

https://hg.mozilla.org/integration/mozilla-inbound/rev/26fa5c01d1a13ffb8999a3ac318737d272f05e6d
https://hg.mozilla.org/integration/mozilla-inbound/rev/1351044edbaa5a08208af54447e38a885321566d

Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=c5dbfdc1892e6003fb128ea305bcf2eb01158c27&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=110829813&repo=mozilla-inbound

 /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/layers/WebRenderBridgeParent.h:235:1: error: version control conflict marker in file
/home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/layers/WebRenderBridgeParent.h:235:1: error: version control conflict marker in file
/home/worker/workspace/build/src/gfx/layers/wr/WebRenderBridgeParent.cpp:393:24: error: out-of-line definition of 'HandleDPEnd' does not match any declaration in 'mozilla::layers::WebRenderBridgeParent'
/home/worker/workspace/build/src/gfx/layers/wr/WebRenderBridgeParent.cpp:521:54: error: too many arguments to function call, expected 9, have 10
/home/worker/workspace/build/src/gfx/layers/wr/WebRenderBridgeParent.cpp:541:54: error: too many arguments to function call, expected 9, have 10

Comment 25

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 - Tab move between different windows when e10s is enabled

Rebased.

Comment 26

[Sotaro Ikeda [:sotaro]]

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

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3bc8240fce38
Tab move between different windows does not work when e10s is enabled r=nical
https://hg.mozilla.org/integration/mozilla-inbound/rev/b60b291fce96
Manually call DidComposite() when transaction was dropped because of obsoleted command r=nical

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3bc8240fce38
https://hg.mozilla.org/mozilla-central/rev/b60b291fce96