New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 824062 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

WASM Rendering broken

Reported by cfakhrud...@zynga.com, Mar 21 2018

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.33 Safari/537.36

Steps to reproduce the problem:
1. Open https://skypebot.farm2.zynga.com/
2. On Mac, on Google Chrome 66.0.3359.33 the page does not show a loader bar

What is the expected behavior?
On Chrome Stable 65 the page loads with a loader bar below

What went wrong?
Rendering is broken with WASM

Did this work before? Yes Stable 65.0.3325.181

Does this work in other browsers? Yes

Chrome version: 66.0.3359.33  Channel: beta
OS Version: OS X 10.12.6
Flash Version: 29.0.0.113

This is issue is reproducible only on Mac and not on Windows.
 
Components: Blink>JavaScript>WebAssembly

Comment 2 by titzer@chromium.org, Mar 21 2018

This is the behavior I see on Stable, Canary, and Chromium latest build:

Stable: hangs with white screen which has some kind of green/white loading bar at the bottom.
Canary and Chromium: black screen, then after a few seconds a blue screen with rendered clouds of some kind.

I didn't test Beta, since it running it side-by-side with stable isn't a supported workflow. What exactly do you see with Beta on Mac?

Comment 3 by titzer@chromium.org, Mar 21 2018

Ok, on Chromium Beta I see the blue screen with white clouds. Is this what you are seeing?
Seeing the white screen and loading bar (green) at the bottom is the right behavior. The sky and clouds are actually a layer behind and they aren't visible. This behavior is correct on the stable version.

On Beta, i can see only the background (ie the clouds and sky screen), the white screen and the loading bar has disappeared on Beta on Mac. I haven't checked on Canary and Chrome Dev build.
@titzer, your observations are same as mine.
This bug in the Beta version breaks our production application completely. Certain things dont render at all due to this.

Comment 7 by titzer@chromium.org, Mar 21 2018

Status: Available (was: Unconfirmed)

Comment 8 by titzer@chromium.org, Mar 21 2018

Components: Blink
I tried with --nowasm-jit-to-native on Chromium Dev66, the most likely culprit, and that has no effect, so I suspect that this is not a WASM bug but a rendering bug.


Components: -Blink Blink>Loader
titzer@
My application is running on WASM and when i run the application on the Flash Player (instead of WASM), it seems to render correctly. Hence, i marked the bug thinking it might be related to WASM.
c10: No worries. Thanks for the report. I hope we can get it worked out soon.
Hi guys, any update on this? This issue still exists.
Cc: japhet@chromium.org yhirano@chromium.org
CC'ing a few Blink>Loader owners for further triaging.
Cc: mtrofin@chromium.org
I'm not sure if this is a loading problem. It would be great if someone having WASM knowledge can look into the issue.
Cc: -mtrofin@chromium.org ahaas@chromium.org
-mtrofin, +ahaas
Cc: abdulsyed@chromium.org ligim...@chromium.org bustamante@chromium.org
Labels: -Pri-2 ReleaseBlock-Stable M-66 FoundIn-66 Needs-Bisect Pri-1
Getting a bisect here would be valuable.

Adding ReleaseBlock-Stable, since there may be a issue w/ layout/ rendering (should revisit once the bisect is complete).
Cc: lafo...@chromium.org
Cc: krajshree@chromium.org ajha@chromium.org
Rajshree, could you please triage and bisect.
we tried bisecting using the public tool,  got the below commit

Downloading revision 535065...
Received 78776576 of 78776576 bytes, 100.00%
Bisecting range [530377 (good), 540271 (bad)], roughly 11 steps left.
Trying revision 535065...
Revision 535065 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b
Downloading revision 532443...
Bisecting range [530377 (good), 535065 (bad)], roughly 10 steps left.
Trying revision 532443...
Revision 532443 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g
Downloading revision 534102...
Bisecting range [532443 (good), 535065 (bad)], roughly 9 steps left.
Trying revision 534102...
Revision 534102 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g
Downloading revision 534402...
Bisecting range [534102 (good), 535065 (bad)], roughly 8 steps left.
Trying revision 534402...
Revision 534402 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g
Downloading revision 534700...
Bisecting range [534402 (good), 535065 (bad)], roughly 7 steps left.
Trying revision 534700...
Revision 534700 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g
Downloading revision 534931...
Bisecting range [534700 (good), 535065 (bad)], roughly 6 steps left.
Trying revision 534931...
Revision 534931 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b
Downloading revision 534828...
Bisecting range [534700 (good), 534931 (bad)], roughly 5 steps left.
Trying revision 534828...
Revision 534828 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g
Downloading revision 534884...
Bisecting range [534828 (good), 534931 (bad)], roughly 4 steps left.
Trying revision 534884...
Revision 534884 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b
Downloading revision 534855...
Bisecting range [534828 (good), 534884 (bad)], roughly 3 steps left.
Trying revision 534855...
Revision 534855 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g
Downloading revision 534864...
Bisecting range [534855 (good), 534884 (bad)], roughly 2 steps left.
Trying revision 534864...
Revision 534864 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g
Downloading revision 534871...
Bisecting range [534864 (good), 534884 (bad)], roughly 2 steps left.
Trying revision 534871...
Revision 534871 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g
You are probably looking for a change made after 534871 (known good), but no later than 534884 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/7adfb7752591c843ef6c90d15b9449d698d8cdea..a02af9067611c9dbe93ace2826eef7e9cd0adb05

^^^^^
Loading bar visible is GOOD
Blue clouds visible is BAD

also note that the stuff is visible once the Preloader.swf gets downloaded(which takes sometime ...roughly 10-15 seconds...you can monitor on devtools network console)
Labels: -Needs-Bisect hasbisect-per-revision Target-67 RegressedIn-66 Target-66 M-67 FoundIn-67 Triaged-ET Needs-Triage-M66
Owner: jmad...@chromium.org
Status: Assigned (was: Available)
Able to reproduce the issue on mac 10.13.3 using chrome reported version #66.0.3359.33 and latest canary #67.0.3387.0.

Bisect Information:
=====================
Good build: 66.0.3341.0
Bad Build : 66.0.3342.0

Change Log URL: 
https://chromium.googlesource.com/chromium/src/+log/6df8a50cc64b41c7018b0415561e2ef9eba085c0..3f5caf560324799038d66714118322ef1b238036

Skia change log:
https://chromium.googlesource.com/angle/angle.git/+log/bfeed4ddfcd6..57dd97aaaf09

From the above change log suspecting below change
Change-Id: I49cbd77328c01f945d66f92e6ec4ba7c552abeff
Reviewed-on: https://chromium-review.googlesource.com/904684

jmadill@ - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.
Note: Adding stable blocker for M-66 as it seems to be a recent regression. Please feel free to remove the same if not appropriate.

Thanks...!!
Cc: jmad...@chromium.org
Owner: ----
Status: Untriaged (was: Assigned)
Change almost certainly couldn't be implicated, it only changes functions in the Vulkan back-end that isn't shipped. The bisect was probably done incorrectly.
I have bisected again and got the same CHANGE LOG (which is different from krajshree)
So the bisect was indeed wrong

Can you please use my results 
05003A-SNAVELKAR:CHROME-BISECT snavelkar$ python bisect-builds.py -a mac64  -g 534871  -b 534950 --use-local-cache
Downloading list of known revisions...
Loaded revisions 15734-547993 from /Users/snavelkar/Documents/CHROME-BISECT/.bisect-builds-cache.json
Downloading revision 534921...
Received 78790868 of 78790868 bytes, 100.00%
Bisecting range [534871 (good), 534949 (bad)], roughly 4 steps left.
Trying revision 534921...
Revision 534921 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b
Downloading revision 534900...
Bisecting range [534871 (good), 534921 (bad)], roughly 3 steps left.
Trying revision 534900...
Revision 534900 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b
Downloading revision 534884...
Bisecting range [534871 (good), 534900 (bad)], roughly 2 steps left.
Trying revision 534884...
Revision 534884 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b
You are probably looking for a change made after 534871 (known good), but no later than 534884 (first known bad).

python bisect-builds.py -a mac64  -g 534864  -b 534888 --use-local-cache
Downloading list of known revisions...
Loaded revisions 15734-547993 from /Users/snavelkar/Documents/CHROME-BISECT/.bisect-builds-cache.json
Downloading revision 534871...
Received 78793974 of 78793974 bytes, 100.00%
Bisecting range [534864 (good), 534884 (bad)], roughly 2 steps left.
Trying revision 534871...
Revision 534871 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g
You are probably looking for a change made after 534871 (known good), but no later than 534884 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/7adfb7752591c843ef6c90d15b9449d698d8cdea..a02af9067611c9dbe93ace2826eef7e9cd0adb05

^^^^^^This is the same CHANGE LOG url I had got earlier as well
It has an ANGLE Chromium change at
https://chromium.googlesource.com/chromium/src/+/3f5caf560324799038d66714118322ef1b238036


Cc: kbr@chromium.org
@Ken - Could this be related to the Angle roll (not necessarily Jamie's CL)?  If it was a generalized rendering issue I'd expect it to be present on Windows too.

@Sarvesh - Thank you for the bisect.  To confirm, this issue is only apparent on MacOS?

Comment 27 by kbr@chromium.org, Apr 4 2018

Cc: cwallez@chromium.org oetu...@nvidia.com
Components: Internals>GPU>ANGLE
Labels: Needs-Bisect
Seems unlikely but could be for example https://chromium.googlesource.com/angle/angle.git/+/fbb1c792150a8d304f20b84a4bf002599be7d97f , which changed the shader translator, which is used on all platforms.

We did check on Windows 10, the issue is not reproducible there 
As an additional data point: all Chrome versions, including M65, fail on Windows when I try them out on BrowserStack (the clouds are always shown instead of the progress bar), but I get the reported results (i.e. all versions work fine) when I try things out on my Windows 10 laptop (I've tried stable, beta and canary).

The difference's that BrowserStack uses SwiftShader with no hardware acceleration features whatsoever. I'm able to reproduce the issue at will on Windows and Linux if I run chrome with --use-gl=swiftshader.

Comment 30 by kbr@chromium.org, Apr 5 2018

Owner: kainino@chromium.org
Kai, could you please confirm the bisect and see whether reverting https://chromium.googlesource.com/angle/angle.git/+/fbb1c792150a8d304f20b84a4bf002599be7d97f locally fixes the problem? Thanks.

You probably need to roll back ANGLE completely to be able to test whether the ImmutableString patch is at fault - there's heaps of code built on top of it now. I doubt that it's the root cause of the issue though.
Labels: -Needs-Bisect
Re-ran the per-revision bisect script considering good and bad builds as in comment #22.
Good behavior is a progressive green bar seen at the bottom of page when loading.
Bad behavior is a blue background with white clouds with no progressive green bar.

Again after running the per-revision bisect script for two times got the same Angle CL as in comment #22 i.e https://chromium.googlesource.com/angle/angle.git/+log/bfeed4ddfcd6..57dd97aaaf09.

Hence, removing Needs-Bisect label.

Thanks...!!
I noticed that this patch in the bisect result may have actually introduced a bug:

https://chromium.googlesource.com/angle/angle.git/+/ca5c105982c0423a80fd1d2f8b73f633ba34f811%5E%21/#F0

According to http://en.cppreference.com/w/cpp/string/multibyte/mbsrtowcs , mbsrtowcs returns static_cast<std::size_t>(-1) on error. The ANGLE code changed in the patch on the other hand seems to treat 0 as an error value. The utility function is only used on D3D in current ANGLE though, so unless the function was used more widely before, this may not be behind the regression.
Components: -Blink>JavaScript>WebAssembly
Status: Started (was: Untriaged)
Investigating.
Owner: oetu...@nvidia.com
Status: Assigned (was: Started)
My per-revision ANGLE bisect does in fact point to
https://chromium.googlesource.com/angle/angle.git/+/fbb1c792150a8d304f20b84a4bf002599be7d97f
specifically.

I don't have any ideas so far as to what the specific issue is.
Tried --disable-webgl-image-chromium and --disable-gpu-compositing but neither works around the issue.

Comment 37 by kbr@chromium.org, Apr 6 2018

Wow, mind blown. We could rebuild Chromium to dump all of the translated shaders and see what changed before and after that patch.

I looked over the patch, paying particular attention to some of the trickier parts like the changes in glsang.l and glslang.y, and could only find one issue:

Instead of writing hex values >= 10 using letters a to f, the character codes were accidentally offset by 10, so it was using letters k to p. This does result in different shader output. I don't know any reason why this would cause any content to misbehave, but I'll fix it either way. Here's a patch: https://chromium-review.googlesource.com/c/angle/angle/+/999480

I don't have a Mac at hand to test this, so Kai could you continue helping with this one?

Agree with kbr's suggested debugging approach if the first patch doesn't do the trick.
Project Member

Comment 39 by bugdroid1@chromium.org, Apr 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/5ae64c94dbf1766d0c9f233942dcfb38c83229ac

commit 5ae64c94dbf1766d0c9f233942dcfb38c83229ac
Author: Olli Etuaho <oetuaho@nvidia.com>
Date: Sat Apr 07 08:48:15 2018

Fix writing hex values in ImmutableStringBuilder

The old code was accidentally using letters offset by 10 when writing
out hex values >= 10. Now the letters a to f are used as they should.

This is one issue that changed shader output when ImmutableString was
introduced, so it is a potential cause for a regression detailed in
 bug 824062 , though this has not been verified.

BUG= chromium:824062 
TEST=angle_unittests

Change-Id: Idb871dffba32a3ab20df0fe17b4b1a98ec00b7fa
Reviewed-on: https://chromium-review.googlesource.com/999480
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>

[modify] https://crrev.com/5ae64c94dbf1766d0c9f233942dcfb38c83229ac/src/compiler/translator/ImmutableStringBuilder.h
[modify] https://crrev.com/5ae64c94dbf1766d0c9f233942dcfb38c83229ac/src/tests/angle_unittests.gypi
[add] https://crrev.com/5ae64c94dbf1766d0c9f233942dcfb38c83229ac/src/tests/compiler_tests/ImmutableString_test.cpp

Project Member

Comment 40 by bugdroid1@chromium.org, Apr 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/37fb7006c59877f6f5bb0772af7385b1999691c0

commit 37fb7006c59877f6f5bb0772af7385b1999691c0
Author: angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Sat Apr 07 11:33:09 2018

Roll src/third_party/angle/ 8957e8328..5ae64c94d (2 commits)

https://chromium.googlesource.com/angle/angle.git/+log/8957e832818b..5ae64c94dbf1

$ git log 8957e8328..5ae64c94d --date=short --no-merges --format='%ad %ae %s'
2018-04-06 oetuaho Fix writing hex values in ImmutableStringBuilder
2018-04-05 lfy GLES1: gl(Push|Pop)Matrix

Created with:
  roll-dep src/third_party/angle
BUG= chromium:824062 


The AutoRoll server is located here: https://angle-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
TBR=fjhenigman@chromium.org

Change-Id: Ia974e589af897c773f9b94355b80d48287e260d1
Reviewed-on: https://chromium-review.googlesource.com/1000554
Reviewed-by: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: angle-chromium-autoroll <angle-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#549056}
[modify] https://crrev.com/37fb7006c59877f6f5bb0772af7385b1999691c0/DEPS

Kai, could you check if the patch fixing the hex output affected the issue on Mac or not? Can you also try with --disable-gpu-shader-disk-cache cmd line option?
The Canary builds unfortunately didn't push this weekend.  We'll need a release >= 67.0.3392.0 to confirm.
Owner: kainino@chromium.org
Without the fix, with --disable-gpu-shader-disk-cache, I still see the bug.

Updating my build to include the patch, will report back.
Labels: Merge-Request-66
Confirming fixed by your patch (#39), thanks Olli!

Though we are close to the M66 stable cut, I'm going to request merge to 66. The impact of this regression (introduced in 66) could be large. (Keeping myself assigned so I can take care of the merge; will reassign it when I close the bug.)

Reviewers: This change (#39) is very small and safe. In fact, this is the full change (excluding new tests):
-            char digitChar = digit < 10 ? digit + '0' : digit + 'a';
+            char digitChar = (digit < 10) ? (digit + '0') : (digit + ('a' - 10));

Unfortunately as pointed out by laforge@, it has not made it to Canary yet.
Project Member

Comment 45 by sheriffbot@chromium.org, Apr 9 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: We are only 7 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Reminder: Please note that M66 Stable is only 7 days away. This bug has been marked as ReleaseBlock Stable for M66. So please take a look and appropriately address this bug. 

Comment 47 by oetu...@nvidia.com, Apr 10 2018

I'm a bit surprised that the small difference in output shader variable naming could cause this type of an issue - are you aware of any code where only using the characters 0 to f is required, or which might throw a tantrum whenever shader output changes? We ruled out the Chromium disk shader cache, but is there some other caching mechanism that might be Mac-specific?

Thanks for looking into still getting the change into M66, I agree it's very low risk.
I checked on Chrome 67.0.3393.0, and looks like the issue is fixed
Labels: -Merge-Review-66 Merge-Approved-66
Approving merge to M66. Branch:3359
Project Member

Comment 50 by bugdroid1@chromium.org, Apr 10 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/22c768fbda545bbba2de6a0728b545bfa7e2ea94

commit 22c768fbda545bbba2de6a0728b545bfa7e2ea94
Author: Olli Etuaho <oetuaho@nvidia.com>
Date: Tue Apr 10 19:29:53 2018

Fix writing hex values in ImmutableStringBuilder

The old code was accidentally using letters offset by 10 when writing
out hex values >= 10. Now the letters a to f are used as they should.

This is one issue that changed shader output when ImmutableString was
introduced, so it is a potential cause for a regression detailed in
 bug 824062 , though this has not been verified.

BUG= chromium:824062 
TEST=angle_unittests

Change-Id: Idb871dffba32a3ab20df0fe17b4b1a98ec00b7fa
Reviewed-on: https://chromium-review.googlesource.com/999480
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
(cherry picked from commit 5ae64c94dbf1766d0c9f233942dcfb38c83229ac)
Reviewed-on: https://chromium-review.googlesource.com/1005800
Reviewed-by: Kai Ninomiya <kainino@chromium.org>

[modify] https://crrev.com/22c768fbda545bbba2de6a0728b545bfa7e2ea94/src/compiler/translator/ImmutableStringBuilder.h
[modify] https://crrev.com/22c768fbda545bbba2de6a0728b545bfa7e2ea94/src/tests/angle_unittests.gypi
[add] https://crrev.com/22c768fbda545bbba2de6a0728b545bfa7e2ea94/src/tests/compiler_tests/ImmutableString_test.cpp

Owner: oetu...@nvidia.com
Status: Fixed (was: Assigned)

Sign in to add a comment