From 6996d1ce369f8335ffa9e303fbcf1c917fe7c3e9 Mon Sep 17 00:00:00 2001
From: Anthony Ramine <n.oxyde@gmail.com>
Date: Sat, 7 Jul 2018 21:52:10 +0200
Subject: [PATCH] Fix gl.getShaderSource and gl.getShaderInfoLog

It only returns null if there was an error, and the only error isn't
implemented yet.
---
 .../script/dom/webglrenderingcontext.rs       |  6 +++--
 components/script/dom/webglshader.rs          | 22 ++++++++-----------
 .../misc/functions-returning-strings.html.ini | 14 ------------
 3 files changed, 13 insertions(+), 29 deletions(-)
 delete mode 100644 tests/wpt/mozilla/meta/webgl/conformance-1.0.3/conformance/misc/functions-returning-strings.html.ini

diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs
index 8ea538c8c92..26b033ae321 100644
--- a/components/script/dom/webglrenderingcontext.rs
+++ b/components/script/dom/webglrenderingcontext.rs
@@ -2527,7 +2527,8 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     fn GetShaderInfoLog(&self, shader: &WebGLShader) -> Option<DOMString> {
-        shader.info_log().map(DOMString::from)
+        // TODO(nox): https://github.com/servo/servo/issues/21133
+        Some(shader.info_log())
     }
 
     #[allow(unsafe_code)]
@@ -2973,7 +2974,8 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     fn GetShaderSource(&self, shader: &WebGLShader) -> Option<DOMString> {
-        shader.source()
+        // TODO(nox): https://github.com/servo/servo/issues/21133
+        Some(shader.source())
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
diff --git a/components/script/dom/webglshader.rs b/components/script/dom/webglshader.rs
index 9b4827e5ce9..1feb73d2d3a 100644
--- a/components/script/dom/webglshader.rs
+++ b/components/script/dom/webglshader.rs
@@ -35,8 +35,8 @@ pub struct WebGLShader {
     webgl_object: WebGLObject,
     id: WebGLShaderId,
     gl_type: u32,
-    source: DomRefCell<Option<DOMString>>,
-    info_log: DomRefCell<Option<String>>,
+    source: DomRefCell<DOMString>,
+    info_log: DomRefCell<DOMString>,
     is_deleted: Cell<bool>,
     attached_counter: Cell<u32>,
     compilation_status: Cell<ShaderCompilationStatus>,
@@ -56,8 +56,8 @@ impl WebGLShader {
             webgl_object: WebGLObject::new_inherited(),
             id: id,
             gl_type: shader_type,
-            source: DomRefCell::new(None),
-            info_log: DomRefCell::new(None),
+            source: Default::default(),
+            info_log: Default::default(),
             is_deleted: Cell::new(false),
             attached_counter: Cell::new(0),
             compilation_status: Cell::new(ShaderCompilationStatus::NotCompiled),
@@ -113,10 +113,6 @@ impl WebGLShader {
         }
 
         let source = self.source.borrow();
-        let source = match source.as_ref() {
-            Some(source) => source,
-            None => return Ok(()),
-        };
 
         let params = BuiltInResources {
             MaxVertexAttribs: limits.max_vertex_attribs as c_int,
@@ -166,7 +162,7 @@ impl WebGLShader {
             },
         };
 
-        match validator.compile_and_translate(&[source]) {
+        match validator.compile_and_translate(&[&source]) {
             Ok(translated_source) => {
                 debug!("Shader translated: {}", translated_source);
                 // NOTE: At this point we should be pretty sure that the compilation in the paint thread
@@ -182,7 +178,7 @@ impl WebGLShader {
             },
         }
 
-        *self.info_log.borrow_mut() = Some(validator.info_log());
+        *self.info_log.borrow_mut() = validator.info_log().into();
 
         // TODO(emilio): More data (like uniform data) should be collected
         // here to properly validate uniforms.
@@ -220,18 +216,18 @@ impl WebGLShader {
     }
 
     /// glGetShaderInfoLog
-    pub fn info_log(&self) -> Option<String> {
+    pub fn info_log(&self) -> DOMString {
         self.info_log.borrow().clone()
     }
 
     /// Get the shader source
-    pub fn source(&self) -> Option<DOMString> {
+    pub fn source(&self) -> DOMString {
         self.source.borrow().clone()
     }
 
     /// glShaderSource
     pub fn set_source(&self, source: DOMString) {
-        *self.source.borrow_mut() = Some(source);
+        *self.source.borrow_mut() = source;
     }
 
     pub fn successfully_compiled(&self) -> bool {
diff --git a/tests/wpt/mozilla/meta/webgl/conformance-1.0.3/conformance/misc/functions-returning-strings.html.ini b/tests/wpt/mozilla/meta/webgl/conformance-1.0.3/conformance/misc/functions-returning-strings.html.ini
deleted file mode 100644
index a3228d706a4..00000000000
--- a/tests/wpt/mozilla/meta/webgl/conformance-1.0.3/conformance/misc/functions-returning-strings.html.ini
+++ /dev/null
@@ -1,14 +0,0 @@
-[functions-returning-strings.html]
-  type: testharness
-  [WebGL test #0: gl.getShaderSource(vs) should return a string.  Returns: "null"]
-    expected: FAIL
-
-  [WebGL test #1: gl.getShaderInfoLog(vs) should return a string.  Returns: "null"]
-    expected: FAIL
-
-  [WebGL test #4: gl.getShaderSource(fs) should return a string.  Returns: "null"]
-    expected: FAIL
-
-  [WebGL test #5: gl.getShaderInfoLog(fs) should return a string.  Returns: "null"]
-    expected: FAIL
-