Ports refactoring #20228

paulrouget · 2018-03-07T10:08:09Z

Depends on #20071 and #19895

This PR does 3 things:

merge all the embedder coordinates callbacks into one
hand the embedder messages directly to the embedder
split the embedder code in 2 files: window.rs and browser.rs

This is in preparation for tabs support in ports/. We want to separate the windowing logic (glutin stuff) and the browsing logic (tabs management, browsers state, etc). It's also easier to bypass the callbacks and directly handle events.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #__ (github issue number if applicable).

There are tests for these changes OR
These changes do not require tests because _____

This change is

highfive · 2018-03-07T10:08:15Z

Heads up! This PR modifies the following files:

@asajeffrey: components/webdriver_server/lib.rs, components/script/dom/screen.rs, components/constellation/constellation.rs, components/script/dom/window.rs
@cbrewster: components/constellation/constellation.rs
@jgraham: components/webdriver_server/lib.rs
@fitzgen: components/script/dom/screen.rs, components/script_traits/script_msg.rs, components/script/dom/window.rs, components/script_traits/lib.rs
@KiChjang: components/script/dom/screen.rs, components/script_traits/script_msg.rs, components/script/dom/window.rs, components/script_traits/lib.rs

highfive · 2018-03-07T10:08:16Z

Warning

These commits modify unsafe code. Please review it carefully!
These commits modify script code, but no tests are modified. Please consider adding a test!

bors-servo · 2018-03-08T20:01:42Z

☔ The latest upstream changes (presumably #20245) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2018-03-16T18:47:22Z

☔ The latest upstream changes (presumably #20315) made this pull request unmergeable. Please resolve the merge conflicts.

paulrouget · 2018-03-17T09:55:44Z

components/servo/lib.rs

+                (EmbedderMsg::KeyEvent(top_level_browsing_context, ch, key, state, modified),
+                 ShutdownState::NotShuttingDown) => {
+                    if state == KeyState::Pressed {
+                        let msg = EmbedderMsg::KeyEvent(top_level_browsing_context, ch, key, state, modified);


Is there a way to not re-construct the event here?

If we did not have to check the state, we could match (msg @ EmbedderMsg::KeyEvent(..), ShutdownState::NotShuttingDown); otherwise we would need to match a clone of the message and push the original.

paulrouget · 2018-03-17T10:02:04Z

@jdm r?

paulrouget · 2018-03-19T04:58:04Z

@bors-servo try

not sure what's happening with Travis: "There was an error while trying to fetch the log."

bors-servo · 2018-03-19T04:58:09Z

⌛ Trying commit 1533372 with merge 53a74a4...

Ports refactoring Depends on #20071 and #19895 This PR does 3 things: 1) merge all the embedder coordinates callbacks into one 2) hand the embedder messages directly to the embedder 3) split the embedder code in 2 files: window.rs and browser.rs This is in preparation for tabs support in `ports/`. We want to separate the windowing logic (glutin stuff) and the browsing logic (tabs management, browsers state, etc). It's also easier to bypass the callbacks and directly handle events. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20228)

paulrouget · 2018-03-22T06:20:07Z

@jdm all green. Want to look at the last commit?

… window coordinates into one structure

jdm · 2018-03-22T12:10:42Z

@bors-servo r+

bors-servo · 2018-03-22T12:10:42Z

📌 Commit 18a02f8 has been approved by jdm

bors-servo · 2018-03-22T12:10:47Z

⌛ Testing commit 18a02f8 with merge 2de8937...

Ports refactoring Depends on #20071 and #19895 This PR does 3 things: 1) merge all the embedder coordinates callbacks into one 2) hand the embedder messages directly to the embedder 3) split the embedder code in 2 files: window.rs and browser.rs This is in preparation for tabs support in `ports/`. We want to separate the windowing logic (glutin stuff) and the browsing logic (tabs management, browsers state, etc). It's also easier to bypass the callbacks and directly handle events. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20228)

bors-servo · 2018-03-22T12:50:11Z

💔 Test failed - mac-rel-wpt4

jdm · 2018-03-22T13:08:21Z

@bors-servo retry

Intermittent timeout in /html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/serialization-via-idb.any.worker.html #20385

bors-servo · 2018-03-22T13:08:24Z

⚡ Previous build results for android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, windows-msvc-dev are reusable. Rebuilding only mac-rel-wpt4...

bors-servo · 2018-03-22T13:51:09Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: jdm
Pushing 2de8937 to master...

highfive assigned metajack Mar 7, 2018

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 7, 2018

paulrouget changed the title ~~[WIP] Ports refactor~~ [WIP] Ports refactoring Mar 7, 2018

highfive added the S-needs-rebase There are merge conflict errors. label Mar 8, 2018

paulrouget force-pushed the ports_refactor branch from d5c5d07 to e840002 Compare March 9, 2018 03:22

jdm assigned jdm and unassigned metajack Mar 9, 2018

jdm added S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. S-needs-rebase There are merge conflict errors. and removed S-needs-rebase There are merge conflict errors. S-awaiting-review There is new code that needs to be reviewed. labels Mar 9, 2018

paulrouget force-pushed the ports_refactor branch from e840002 to 37b6fc4 Compare March 16, 2018 10:09

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 16, 2018

highfive added the S-needs-rebase There are merge conflict errors. label Mar 16, 2018

paulrouget force-pushed the ports_refactor branch 2 times, most recently from fd2c0e7 to 1533372 Compare March 17, 2018 09:50

paulrouget commented Mar 17, 2018

View reviewed changes

paulrouget changed the title ~~[WIP] Ports refactoring~~ Ports refactoring Mar 17, 2018

paulrouget mentioned this pull request Mar 19, 2018

Tab support (UI) #20335

Open

jdm removed S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. S-needs-rebase There are merge conflict errors. labels Mar 19, 2018

paulrouget force-pushed the ports_refactor branch from f0387fd to e77ac1e Compare March 22, 2018 05:22

paulrouget added 7 commits March 22, 2018 07:56

Reduce the number of calls to the embedder by grouping the screen and…

10abe03

… window coordinates into one structure

forward EmbedderMsg to embedder

5e33dcd

move key code into dedicated file

34c2150

split window code and browser code in two different files

1cdba88

remove useless pref

aa72b87

Use the --device-pixel-ratio in opt instead of /components/servo/

abea15d

loop until we exhausted all the servo events

18a02f8

paulrouget force-pushed the ports_refactor branch from e77ac1e to 18a02f8 Compare March 22, 2018 07:07

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. S-needs-rebase There are merge conflict errors. labels Mar 22, 2018

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Mar 22, 2018

bors-servo merged commit 18a02f8 into servo:master Mar 22, 2018

Android MVP automation moved this from In progress to Done Mar 22, 2018

bors-servo mentioned this pull request Mar 22, 2018

Profiler for blocked IpcReceiver::recv() #20132

Merged

4 tasks

This was referenced Mar 28, 2018

Issues with font rendering paulrouget/servo-embedding-example#11

Open

Update example paulrouget/servo-embedding-example#12

Open

Layout on windows 10 differ between dpi scaling #20505

Open

kwonoj mentioned this pull request Apr 4, 2018

fix(window): apply dpi aware viewport #20522

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ports refactoring #20228

Ports refactoring #20228

paulrouget commented Mar 7, 2018 •

edited

highfive commented Mar 7, 2018

highfive commented Mar 7, 2018

bors-servo commented Mar 8, 2018

bors-servo commented Mar 16, 2018

paulrouget Mar 17, 2018

jdm Mar 19, 2018

paulrouget commented Mar 17, 2018

paulrouget commented Mar 19, 2018 •

edited

bors-servo commented Mar 19, 2018

paulrouget commented Mar 22, 2018

jdm commented Mar 22, 2018

bors-servo commented Mar 22, 2018

bors-servo commented Mar 22, 2018

bors-servo commented Mar 22, 2018

jdm commented Mar 22, 2018

bors-servo commented Mar 22, 2018

bors-servo commented Mar 22, 2018

Ports refactoring #20228

Ports refactoring #20228

Conversation

paulrouget commented Mar 7, 2018 • edited

highfive commented Mar 7, 2018

highfive commented Mar 7, 2018

bors-servo commented Mar 8, 2018

bors-servo commented Mar 16, 2018

paulrouget Mar 17, 2018

Choose a reason for hiding this comment

jdm Mar 19, 2018

Choose a reason for hiding this comment

paulrouget commented Mar 17, 2018

paulrouget commented Mar 19, 2018 • edited

bors-servo commented Mar 19, 2018

paulrouget commented Mar 22, 2018

jdm commented Mar 22, 2018

bors-servo commented Mar 22, 2018

bors-servo commented Mar 22, 2018

bors-servo commented Mar 22, 2018

jdm commented Mar 22, 2018

bors-servo commented Mar 22, 2018

bors-servo commented Mar 22, 2018

paulrouget commented Mar 7, 2018 •

edited

paulrouget commented Mar 19, 2018 •

edited