It's pretty messy, and could use a bit of a cleanup. Particularly, it'd be nice to improve readability of code flow regarding which parts can cause context creation to fail.

From 9d86dbd546df9349a0a9f1c089b491a7524e9450 Mon Sep 17 00:00:00 2001 --- dom/canvas/WebGLContext.cpp | 2 +- gfx/gl/GLContext.cpp | 1934 ++++++++++++++++++--------------------- gfx/gl/GLContext.h | 29 +- gfx/gl/GLContextCGL.h | 2 +- gfx/gl/GLContextEAGL.h | 2 +- gfx/gl/GLContextProviderCGL.mm | 6 - gfx/gl/GLContextProviderEAGL.mm | 6 - gfx/gl/GLLibraryLoader.cpp | 6 +- gfx/gl/GLLibraryLoader.h | 4 +- 9 files changed, 897 insertions(+), 1094 deletions(-) Review commit: https://reviewboard.mozilla.org/r/44399/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/44399/

Comment on attachment 8738264 [details] MozReview Request: Bug 1262265 - Cleanup InitWithPrefix. - r?jrmuizel https://reviewboard.mozilla.org/r/44399/#review42077 It's probably worth adding gfxCriticalNote/gfxCriticalWarning's in the places that are not expected to fail but we still attempt to handle failure case.

(In reply to Jeff Muizelaar [:jrmuizel] from comment #2) > Comment on attachment 8738264 [details] > MozReview Request: Bug 1262265 - Cleanup InitWithPrefix. - r?jrmuizel > > https://reviewboard.mozilla.org/r/44399/#review42077 > > It's probably worth adding gfxCriticalNote/gfxCriticalWarning's in the > places that are not expected to fail but we still attempt to handle failure > case. e.g. when we get the renderer string etc.

Comment on attachment 8738264 [details] MozReview Request: Bug 1262265 - Cleanup InitWithPrefix. - r?jrmuizel https://reviewboard.mozilla.org/r/44399/#review42117

Backed out for mass android bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=25561389&repo=mozilla-inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/cc130a8f204d

Not just android, as it turns out: https://treeherder.mozilla.org/logviewer.html#?job_id=25562779&repo=mozilla-inbound

From 30a8c48ef9afeb8cac5124244a1f4091d0a2238d Mon Sep 17 00:00:00 2001 --- gfx/gl/GLContextProviderCGL.mm | 19 +++++++------------ gfx/gl/GLContextProviderEAGL.mm | 13 ++++++++----- gfx/gl/GLContextProviderGLX.cpp | 15 ++++----------- gfx/gl/GLContextProviderWGL.cpp | 20 +++----------------- 4 files changed, 22 insertions(+), 45 deletions(-) Review commit: https://reviewboard.mozilla.org/r/45915/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45915/

From 6aa28e18fb9ff96e22945d130933ba6062161372 Mon Sep 17 00:00:00 2001 --- gfx/gl/GLContextProviderEGL.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) Review commit: https://reviewboard.mozilla.org/r/45917/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45917/

From 6ea76a49d344a5e122709b4cffa75a5a618e7fe1 Mon Sep 17 00:00:00 2001 --- gfx/gl/GLContextProviderEGL.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) Review commit: https://reviewboard.mozilla.org/r/45919/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45919/

From 649a327d8a82cb3b7339223174a787b6957ae2b8 Mon Sep 17 00:00:00 2001 --- gfx/gl/GLContextProviderCGL.mm | 3 ++- gfx/gl/GLContextProviderEAGL.mm | 3 ++- gfx/gl/GLContextProviderGLX.cpp | 3 ++- gfx/gl/GLContextProviderWGL.cpp | 5 +++-- 4 files changed, 9 insertions(+), 5 deletions(-) Review commit: https://reviewboard.mozilla.org/r/45921/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45921/

From 7c9bbc1952be0da6c6d7b2d745edb4a05a26715a Mon Sep 17 00:00:00 2001 --- gfx/gl/GLContext.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Review commit: https://reviewboard.mozilla.org/r/45923/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45923/

Comment on attachment 8738264 [details] MozReview Request: Bug 1262265 - Cleanup InitWithPrefix. - r?jrmuizel Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44399/diff/1-2/

Can't we just get rid of GetGlobalContext from the public GLContextProvider API? There aren't any callers of it. GLContextProviderCGL doesn't even implement it usefully, context sharing is never enabled for CGL [1] . I doubt the other backends really need context sharing either. [1] http://mxr.mozilla.org/mozilla-central/source/gfx/gl/GLContextProviderCGL.mm#235

(In reply to Matt Woodrow (:mattwoodrow) from comment #14) > Can't we just get rid of GetGlobalContext from the public GLContextProvider > API? > > There aren't any callers of it. > > GLContextProviderCGL doesn't even implement it usefully, context sharing is > never enabled for CGL [1] . > > I doubt the other backends really need context sharing either. > > > > [1] > http://mxr.mozilla.org/mozilla-central/source/gfx/gl/GLContextProviderCGL. > mm#235 F+, but I think EAGL uses it actually. Snorp: Does iOS rely on context sharing via GetGlobalContext?

Comment on attachment 8740690 [details] MozReview Request: Simplify and standardize GetGlobalContext. r?jrmuizel https://reviewboard.mozilla.org/r/45915/#review43449

I backed out d3aab3c4eb5f (comment 17). Backout cset is: https://hg.mozilla.org/integration/mozilla-inbound/rev/1a1420dcc987 Backout was for GL-related fatal assertion failures like: Assertion failure: internalFormat == 0x1908, at c:/builds/moz2_slave/m-in-w32-d-0000000000000000000/build/src/gfx/gl/GLContext.cpp:2925 Sample log: https://treeherder.mozilla.org/logviewer.html#?job_id=26164012&repo=mozilla-inbound And there are more of these on the inbound treeherder run where this patch had landed: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=d3aab3c4eb5fae8b7768438577c95b0dc364e215

From 34a6b4b4db28ebec0d0ceb021a418117a8c89444 Mon Sep 17 00:00:00 2001 --- gfx/gl/GLContextProviderEGL.cpp | 4 ++++ gfx/gl/GLLibraryEGL.h | 3 +++ 2 files changed, 7 insertions(+) Review commit: https://reviewboard.mozilla.org/r/47943/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47943/

Comment on attachment 8738264 [details] MozReview Request: Bug 1262265 - Cleanup InitWithPrefix. - r?jrmuizel Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44399/diff/2-3/

Comment on attachment 8740690 [details] MozReview Request: Simplify and standardize GetGlobalContext. r?jrmuizel Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45915/diff/1-2/

Comment on attachment 8740691 [details] MozReview Request: Centralize EnsureInitialized. r?jrmuizel Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45917/diff/1-2/

Comment on attachment 8740692 [details] MozReview Request: Don't call Init() twice. r?jrmuizel Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45919/diff/1-2/

Comment on attachment 8740693 [details] MozReview Request: Compiler says 'no'. r?jrmuizel Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45921/diff/1-2/

Comment on attachment 8740694 [details] MozReview Request: Fix the conditional. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45923/diff/1-2/

Comment on attachment 8740691 [details] MozReview Request: Centralize EnsureInitialized. r?jrmuizel https://reviewboard.mozilla.org/r/45917/#review44899

Comment on attachment 8740692 [details] MozReview Request: Don't call Init() twice. r?jrmuizel https://reviewboard.mozilla.org/r/45919/#review44901

Comment on attachment 8740693 [details] MozReview Request: Compiler says 'no'. r?jrmuizel https://reviewboard.mozilla.org/r/45921/#review44903

Comment on attachment 8740694 [details] MozReview Request: Fix the conditional. https://reviewboard.mozilla.org/r/45923/#review44905

Comment on attachment 8743624 [details] MozReview Request: r?jrmuizel - Require initialization for egl.IsANGLE(). https://reviewboard.mozilla.org/r/47943/#review45033

(In reply to Jeff Gilbert [:jgilbert] from comment #15) > (In reply to Matt Woodrow (:mattwoodrow) from comment #14) > > Can't we just get rid of GetGlobalContext from the public GLContextProvider > > API? > > > > There aren't any callers of it. > > > > GLContextProviderCGL doesn't even implement it usefully, context sharing is > > never enabled for CGL [1] . > > > > I doubt the other backends really need context sharing either. > > > > > > > > [1] > > http://mxr.mozilla.org/mozilla-central/source/gfx/gl/GLContextProviderCGL. > > mm#235 > > F+, but I think EAGL uses it actually. > > Snorp: Does iOS rely on context sharing via GetGlobalContext? Sorry for the late reply, but yes, it does use that.