Remove pipeline_size_changed from RenderNotifier and update toml

[hgallagher1993]

r? @jdm

---

This change is 

[jdm]

Did you build after making this change? There are places in the code that call the method that was removed.

[hgallagher1993]

I actually forgot to build it 😦 . . .there was 3 calls to it anyway in render_backend.rs, I'm just waiting for a file lock to lift before it'll build

[hgallagher1993]

Ya after just removing those 3 calls to pipeline_size_changed webrender compiles but not Servo because

fn pipeline_size_changed(&mut self,_: webrender_traits::PipelineId,_: Option<webrender_traits::LayoutSize>) { }

is still in the impl in compositor.rs, everything compiled fine when I removed that. So that should be removed fairly soon after this pull request is finished should it?

[jdm]

Yep.

[hgallagher1993]

If this is ok do you want me to squash the commit?

[jdm]

This code doesn't do anything now, since it isn't used to notify anything. We can remove all of the code that compares the sizes of pipelines and stores them for later.

[hgallagher1993]

Sorry I'm just struggling to get this, it's the first time I've ever seen this code :-/, Is it basically everything to do with updated_pipeline_sizes that can be removed? So pretty much everything bellow self.frame.create(&self.scene, &mut new_pipeline_sizes);

[jdm]

As far as I know, the only use of storing the known pipeline sizes is to compare them in this code. Since we don't need to notify any other code about changes to pipeline sizes, that means we can get rid of all of the code that compares them, as well as any code that stores them for later use. Basically, any code that now does not do anything with the result of comparisons can be removed.

[jdm]

Squashing commits is fine.

[hgallagher1993]

I can make the change to Servo as well if you want

[glennw]

@jdm The Servo code that uses this feature is being reverted due to causing test failures, I believe (see servo/servo#15164). If that's right, we should hold off on this until the Servo code changes land without causing regressions?

[jdm]

I'm inclined to hold off on merging it, yes. But I think working on it before then is fine.

[glennw]

Yes, that's fine - it may just involve a lot of rebasing the patch, depending on how long it takes to get the Servo changes to land.

[emilio]

I think the servo changes were legitimately green on try, so should be easy to reland them I hope.

…

On Tue, Jan 24, 2017 at 11:25:31AM -0800, Glenn Watson wrote: Yes, that's fine - it may just involve a lot of rebasing the patch, depending on how long it takes to get the Servo changes to land. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: #772 (comment)

[glennw]

I think the Servo changes have landed now? However, there are CI compile failures for this PR that need to be resolved first.

[hgallagher1993]

I think it's just the Cargo.lock I have to commit to get it compiling, it's compiling for me locally now like, just have to makes the changes jdm requested first

[jdm]

I will defer to the proper reviewers for this code, but that looks more like what I expected to see :)

[glennw]

This looks good - there's a couple CI errors to fix though. If you cd wrench && cargo build and then cd sample && cargo build you should be able to see the errors to fix locally.

[hgallagher1993]

Wrench and the sample are building now

[emilio]

This looks good, but could you remove the merge commits, please?

[hgallagher1993]

To remove the 2 commits is it just git revert -m 1 [sha] then commit and push the reverts? I've been reading up on reverting merges and I've found a few different answers. . . . .This is the answer I'm going off atm

[emilio]

You can do git rebase -i HEAD~5, then remove the lines corresponding to merge commits.

[bors-servo]

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

[glennw]

Waiting on squash and rebase, then this should be good to go.

[hgallagher1993]

Just on the merges I've to remove, I ran git rebase -i HEAD~5 but the 2 merge commits don't appear anywhere in the list, so is there any other way they could be removed?

[emilio]

If you check the "Allow edits from collaborators" checkbox I can do it for you, exporting the patches with git format-patch, then resetting to master, then applying them with git am --3way works, though I'm pretty sure there should be an easier way...

[hgallagher1993]

That's checked now

[hgallagher1993]

The Allow edits from maintainers

[emilio]

Just git fetch upstream and git rebase upstream/master works, if you want to give it a try. Otherwise I can just push the squashed version :)

[hgallagher1993]

Is there any chance you could just push it? because I'm actually just about to go back to a class in college that I'll be in for a few hours, I was only on a break just now 😆

[emilio]

Sure :)

[emilio]

@bors-servo r=glennw

[bors-servo]

📌 Commit f77b5e6 has been approved by glennw

[emilio]

Thanks for working on this @hgallagher1993!

[bors-servo]

⌛ Testing commit f77b5e6 with merge c82191a...

[bors-servo]

☀️ Test successful - status-travis
Approved by: glennw
Pushing c82191a to master...