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

RESOLVED FIXED in Firefox 56

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: sotaro, Assigned: sotaro)

Tracking

(Blocks 1 bug)

unspecified
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(2 attachments, 12 obsolete attachments)

4.75 KB, patch
nical
: review+
Details | Diff | Splinter Review
34.76 KB, patch
sotaro
: review+
Details | Diff | Splinter Review
(Assignee)

Description

2 years ago
Tab move between different windows does not work yet.
(Assignee)

Updated

2 years ago
Blocks: webrender
(Assignee)

Updated

2 years ago
Depends on: 1335336
(Assignee)

Updated

2 years ago
Attachment #8831973 - Flags: review?(nical.bugzilla)
(Assignee)

Updated

2 years ago
Attachment #8831973 - Attachment is obsolete: true
Attachment #8831973 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 2

2 years ago
Posted patch wip patch (obsolete) — Splinter Review
(Assignee)

Comment 3

2 years ago
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.
(Assignee)

Updated

2 years ago
Summary: Tab move between different windows does not work → Tab move between different windows does not work when e10s is enabled.
Duplicate of this bug: 1333762
Whiteboard: [gfx-noted]
(Assignee)

Updated

2 years ago
Assignee: nobody → sotaro.ikeda.g
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1368876
(Assignee)

Updated

2 years ago
Attachment #8831975 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Depends on: 1372066
(Assignee)

Comment 6

2 years ago
Posted patch wip patch (obsolete) — Splinter Review
(Assignee)

Updated

2 years ago
Depends on: 1372512
(Assignee)

Comment 7

2 years ago
Posted patch wip patch (obsolete) — Splinter Review
Attachment #8877033 - Attachment is obsolete: true
(Assignee)

Comment 8

2 years ago
Attachment #8878403 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Attachment #8879500 - Attachment description: Tab move between different windows when e10s is enabled → patch - Tab move between different windows when e10s is enabled
(Assignee)

Comment 10

2 years ago
Fixed crash during tab move.
Attachment #8879500 - Attachment is obsolete: true
(Assignee)

Comment 12

2 years ago
Attachment #8879864 - Attachment is obsolete: true
(Assignee)

Comment 14

2 years ago
Attachment #8880216 - Attachment is obsolete: true
(Assignee)

Comment 15

2 years ago
Attachment #8880259 - Attachment is obsolete: true
(Assignee)

Comment 16

2 years ago
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.
(Assignee)

Updated

2 years ago
Attachment #8880724 - Attachment description: patch - Tab move between different windows when e10s is enabled → patch part 1 - Tab move between different windows when e10s is enabled
(Assignee)

Comment 18

2 years ago
Update a comment.
Attachment #8880724 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Attachment #8880726 - Flags: review?(nical.bugzilla)
(Assignee)

Updated

2 years ago
Attachment #8880733 - Flags: review?(nical.bugzilla)
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.
Attachment #8880726 - Flags: review?(nical.bugzilla) → review+
Attachment #8880733 - Flags: review?(nical.bugzilla) → review+
(Assignee)

Comment 22

2 years ago
Apply the comment.
Attachment #8880726 - Attachment is obsolete: true
Attachment #8882286 - Flags: review+

Comment 23

2 years ago
Pushed by sikeda@mozilla.com:
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
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
Flags: needinfo?(sotaro.ikeda.g)
(Assignee)

Comment 25

2 years ago
Rebased.
Attachment #8882286 - Attachment is obsolete: true
Flags: needinfo?(sotaro.ikeda.g)
Attachment #8882434 - Flags: review+

Comment 27

2 years ago
Pushed by sikeda@mozilla.com:
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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3bc8240fce38
https://hg.mozilla.org/mozilla-central/rev/b60b291fce96
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.