Skip to content

/ servo

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid a panic when closing webgl pages using VAOs #25998

Merged
merged 1 commit into from Mar 27, 2020
Merged

Avoid a panic when closing webgl pages using VAOs #25998

merged 1 commit into from Mar 27, 2020

Conversation

@jdm
Copy link
Member

jdm commented Mar 20, 2020

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #25891
  • These changes do not require tests because GC behaviour at shutdown is nondeterministic and difficult to test
@jdm
webgl: Ignore webgl communication errors when dropping vertex array o…
Loading status checks…
3a3397f 
…bjects.
@jdm
Copy link
Member Author

jdm commented Mar 20, 2020

r? @nox
@highfive
Copy link

highfive commented Mar 20, 2020

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webgl2renderingcontext.rs, components/script/dom/webglrenderingcontext.rs, components/script/dom/vertexarrayobject.rs, components/script/dom/webglbuffer.rs
  • @KiChjang: components/script/dom/webgl2renderingcontext.rs, components/script/dom/webglrenderingcontext.rs, components/script/dom/vertexarrayobject.rs, components/script/dom/webglbuffer.rs
@highfive
Copy link

highfive commented Mar 20, 2020

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@jdm jdm assigned nox and unassigned paulrouget Mar 20, 2020
@jdm
Copy link
Member Author

jdm commented Mar 27, 2020

Review ping @nox.
@nox
nox approved these changes Mar 27, 2020
View changes
Copy link
Member

nox left a comment

Seems good, but IMO we should not propagate those values as booleans.
components/script/dom/webglbuffer.rs
@@ -166,15 +166,15 @@ impl WebGLBuffer {
);
}

pub fn decrement_attached_counter(&self) {
pub fn decrement_attached_counter(&self, fallible: bool) {

This comment has been minimized.

Sign in to view
Copy link
@nox

nox Mar 27, 2020

Member

Not a fan of the boolean argument.

This comment has been minimized.

Sign in to view
Copy link
@jdm

jdm Mar 27, 2020

Author Member

Filed #26047.
@nox
Copy link
Member

nox commented Mar 27, 2020

You didn't not introduce those booleans you just used them in more places, so

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Mar 27, 2020

📌 Commit 3a3397f has been approved by nox
@bors-servo
Copy link
Contributor

bors-servo commented Mar 27, 2020

⌛ Testing commit 3a3397f with merge a927f1a...
@bors-servo
Copy link
Contributor

bors-servo commented Mar 27, 2020

☀️ Test successful - status-taskcluster
Approved by: nox
Pushing a927f1a to master...
@bors-servo bors-servo merged commit a927f1a into servo:master Mar 27, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nox nox

Assignees

@nox nox

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

WebGLVertexArrayObjectOES's destructor shouldn't send infallible webgl commands
5 participants
@jdm @highfive @nox @bors-servo @paulrouget
You can’t perform that action at this time.