New issue
Advanced search Search tips

Issue 605191 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 586183
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

method bind() occasionally returns undefined?

Project Member Reported by klausw@google.com, Apr 20 2016

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.87 Safari/537.36

Steps to reproduce the problem:
1. application contains this code: img.onclick = this.method_.bind(this, stringArg)
2. reload the application repeatedly or navigate within the application
3. click the image, look at onclick value in Chrome inspector

What is the expected behavior?
The onclick handler should be set to the bound method.

What went wrong?
Clicking the image does nothing. The onclick value shows as undefined in inspector, note this is different from the default null value for unassigned handlers.

Did this work before? Yes Chrome 49.0.2623.87 and earlier versions worked fine

Chrome version:  50.0.2661.26  Channel: stable
OS Version: x86_64
Flash Version: 

This definitely seems related to the new Chrome version. At first only one user reported the issue with nobody else on the team being able to reproduce it. Bug was filed Mar 16, local OS logs show that the upgrade to Chrome 50.x happened on Mar 10 as part of the beta channel. Now that Chrome 50 has hit stable channel, it's affecting others too.

The bug also happens in incognito windows, so it's unlikely to be related to Chrome extensions.

Adding a console.log(img.onclick) seems to prevent the issue from happening, haven't been able to reproduce it after adding that. A Chrome inspector watch that checks for assignments to the img element did show undefined values being assigned, hence the hypothesis that bind() sometimes returns undefined.

The symptoms persisted after changing the code to use img.addEventListener('click', this.method.bind(...)). The onclick property now remained null, I think this rules out the possibility that some other code was setting it to undefined elsewhere. There's no 'click' handler shown when the issue occurs, same as when I manually call img.addEventHandler('click', undefined).

Sorry, I don't have a repro case I can share publically. See http://b/27698164 for the internal bug, I can help walk through the steps needed if applicable.

I suspect this may be related to the bind implementation refactor in https://codereview.chromium.org/1542963002 , and a suspicion that the new logic may be triggering a bug in v8.

While searching for related bugs, I found  http://crbug.com/596301  "Objects constructed in another window triggering V8 miscompile(?)". Seems only vaguely similar since the symptoms and repro steps aren't the same, but that also involved bind() and things apparently started breaking starting with Chrome 50.x.

Please let me know if you need additional info from me.
 

Comment 1 by klausw@google.com, Apr 20 2016

For the record, the browser I used to file this bug is still Chrome 49, it's not affected by the issue. Others running 50.x are affected, as noted in the version info in the issue body.
Components: -Blink Blink>JavaScript
Cc: jochen@chromium.org verwa...@chromium.org
Components: -Blink>JavaScript Blink>JavaScript>Runtime
Labels: -Pri-2 Pri-1
Owner: bmeu...@chromium.org
Status: Assigned (was: Unconfirmed)
Requested repro in internal bug  http://b/27698164.

Cc: hablich@chromium.org
Cc: -verwa...@chromium.org
Owner: verwa...@chromium.org

Comment 6 by klausw@google.com, Apr 25 2016

FYI, I've added repro instructions to http://b/27698164#comment12 , note updated starting link in http://b/27698164#comment14 since we added a workaround to the prod version. Let me know if you need more info, or if the repro isn't working for you.
Cc: verwa...@chromium.org
Mergedinto: 586183
Owner: haraken@chromium.org
Status: Duplicate (was: Assigned)
This is a dupe of an issue that was fixed in https://codereview.chromium.org/1724263004. @haraken: you should probably merge back the fix to M50 (at least)?
That CL was already merged into M50.

Sign in to add a comment