Skip to content
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

DDC-built Angular app not working in Chrome Canary #43193

Closed
missvalentinep opened this issue Aug 26, 2020 · 22 comments
Closed

DDC-built Angular app not working in Chrome Canary #43193

missvalentinep opened this issue Aug 26, 2020 · 22 comments
Assignees
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. P1 A high priority bug; for example, a single project is unusable or has many test failures web-dev-compiler

Comments

@missvalentinep
Copy link

When opening in Chrome Canary my Angular app built with DDC I get this error and nothing works.

dart_sdk.js:99588 EXCEPTION: NoSuchMethodError: 'currentIndex'
method not found
Receiver: Instance of 'CollectionChangeRecord'

Full stack trace is available here.

No error is present when opening the same app in ordinary Chrome. Also, opening the app compiled with dart2js in Canary works, too.

I'm not sure who is responsible for this error - AngularDart, DarkSDK or Chrome Canary. I've created an issue in Angular repo but it seems to be a bit dead, so I'm writing to you guys.

Here is the link to the original issue - angulardart/angular#1905, it has a bit more info on what I've managed to find out myself.

@sigmundch sigmundch added area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. web-dev-compiler labels Aug 26, 2020
@sigmundch
Copy link
Member

Thanks @missvalentinep for filing this issue and for all the details in the original bug. We need to investigate more closely to see what's happening here. Please keep us posted too if you notice that this changes on the next canary release.

@sigmundch sigmundch added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Aug 26, 2020
@sigmundch sigmundch added this to the September Release 2020 milestone Aug 26, 2020
@sigmundch
Copy link
Member

@nshahan @Markzipan - could one of you take a closer look? I labeled P1 just in case this is something that will become a bigger problem once chrome releases the changes to beta/stable (like the issue we had with chrome 81).

@Markzipan
Copy link
Contributor

Thanks for reporting! This appears to be a much more general issue, as the error you described in your original bug is reproducible with Angular's Hello World app (on Canary):

NoSuchMethodError: method not found: 'get$digestsPath' (J.getInterceptor$x(...).get$digestsPath is not a function)
TypeError: J.getInterceptor$x(...).get$digestsPath is not a function
    at Object.get$digestsPath$x (http://localhost:8080/dwds/src/injected/client.js:3536:43)
   ...

We're continuing the investigation.

@sigmundch
Copy link
Member

interesting - could we be facing 2 different issues?

The exception in this last comment appears to be affecting the dwds's client compiled with dart2js /cc @rakudrama @grouma

Is the DDC issue reproducible with the hello world app if you run webdev with --no-injected-client ?

@grouma
Copy link
Member

grouma commented Aug 26, 2020

I'll dig in from the webdev side. The tracking similar issue is: dart-lang/webdev#1087

@Markzipan
Copy link
Contributor

Markzipan commented Aug 26, 2020

The bug disappears with --no-injected-client with DDC on Canary. It's likely the same issue as: dart-lang/webdev#1087

@sigmundch
Copy link
Member

@missvalentinep - do you have any pointers or an example you could provide for us to reproduce the .currentIndex error?

@Markzipan
Copy link
Contributor

Markzipan commented Aug 26, 2020

This issue stems from window.performance.memory.constructor no longer returning a MemoryInfo constructor (and instead a standard Object constructor), which breaks some assumptions about how we attach runtime caches in client.js's initNativeDispatchContinue operation. No idea if this is the Chrome team's intent, however.

See: https://github.com/dart-lang/sdk/blob/master/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/runtime.dart#L80

Unless this is somehow a red herring.

@sigmundch
Copy link
Member

interesting... seems like a couple things to look into:

  • if the API is gone, should we remove it from dart:html altogether
  • Is there something in both DDC and dart2js we can do to be more robust when the lookup of the native constructor seems invalid?

/cc @srujzs @rakudrama

@Markzipan
Copy link
Contributor

I can confirm that removing all instances of MemoryInfo from both the DDC runtime and dart:html resolves the issue. This would obviously break anything that currently depends on those, however. If we want to retain access to this, we'd need a new disambiguation structure in the DDC runtime that isn't the JS constructor (since that's now shared with Object's).

@srujzs
Copy link
Contributor

srujzs commented Aug 26, 2020

if the API is gone, should we remove it from dart:html altogether

I don't think MemoryInfo is removed from Chrome (even though it's non-standard) yet. I don't mind the breaking change if necessary, but we'll need a workaround for wherever it's being used. There's a handful of usages in google3 from what I can tell.

@Markzipan
Copy link
Contributor

Yeah, that part's odd. Judging from the Chrome IDL (https://github.com/chromium/chromium/blob/master/third_party/blink/renderer/core/timing/memory_info.idl#L35), MemoryInfo's constructor hasn't been exposed for a while now. Wonder why we're only seeing this issue now.

@missvalentinep
Copy link
Author

missvalentinep commented Aug 27, 2020

do you have any pointers or an example you could provide for us to reproduce the .currentIndex error?

@sigmundch the issue is reproducible in Tour of Heroes 2 - https://github.com/angular-examples/toh-2 (just in case - I'm using webdev 2.5.9 so I rose the build_runner and build_web_compilers versions to minimal required ones).

What's interesting, that it's not reproduced in toh-1. Apparently, the second part adds some complexity that starts to cause the problem.

dart-bot pushed a commit that referenced this issue Aug 28, 2020
…me API changes

Chrome Canary no longer exposes window.performance.memory's constructor (it uses Object's instead). This hack lets (window.performance.memory is html.MemoryInfo) evaluate to true.

See: #43193
Change-Id: I3c557467c3ea029052aba71f6695f4db0ec65515
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/160782
Reviewed-by: Sigmund Cherem <[email protected]>
Commit-Queue: Mark Zhou <[email protected]>
@sigmundch
Copy link
Member

@missvalentinep - @Markzipan landed e33f84d which should fix DDC after this chrome breaking change. Dart2js still needs more work, so this should only unblock the issue during development and with the --no-injected-client tag.

For dart2js we need a very different fix.

@rakudrama - can you share pointers to where we'd need to modify the caching of interceptors to detect that we are not accidentally caching anything on Object.contructor.prototype? For example, would that be on _prototypeForTag in native_helpers?

@missvalentinep
Copy link
Author

@sigmundch is there any way that this problem can be fixed in SDK 2.8? Not everyone is using the latest SDK version and moving to it so rapidly to get this fix can pretty be painful.

@Markzipan
Copy link
Contributor

@sigmundch I can confirm that the DDC fix also fixes the issue for Dart2JS (for the ToH sample). We can close after the SDK versioning is figured out.

@sigmundch
Copy link
Member

Awesome, thanks for following up @Markzipan !

For context @missvalentinep , we discovered that dart2js was working properly, but was tripping on a cascading issue caused by DDC's bug. There is a longer goal to prevent this kind of interference, but for now the fix we landed will be enough to address this problem altogether. Yay!

I'll follow up on a cherry-pick request to get e33f84d on the current stable version. My understanding is that it will get to 2.9, but might not be possible to include it with 2.8. However, it is possible to workaround this by adding a couple lines of JavaScript in the page serving the application with practically the same logic as the change above:

<script>
if (typeof window.MemoryInfo == "undefined") {
  if (typeof window.performance.memory != "undefined") {
    window.MemoryInfo = function () {};
    window.MemoryInfo.prototype = window.performance.memory.__proto__;
  }
}
</script>

@sigmundch
Copy link
Member

And here is the cherry-pick request: #43287

dart-bot pushed a commit that referenced this issue Sep 8, 2020
…me API changes

Chrome Canary no longer exposes window.performance.memory's constructor (it uses Object's instead). This hack lets (window.performance.memory is html.MemoryInfo) evaluate to true.

See: #43193
Change-Id: I3c557467c3ea029052aba71f6695f4db0ec65515
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/160782
Reviewed-by: Sigmund Cherem <[email protected]>
Commit-Queue: Mark Zhou <[email protected]>
@Fox32
Copy link

Fox32 commented Oct 5, 2020

@sigmundch Is it possible that other class beside ´MemoryInfo´ had similar changes? Since upgrading to Chrome 87 I run into multiple strange issues when using DDC. With the above workaround I could resolve one of them, but there are more.

I have some code like this:

import 'dart:js' show context;

...
  context['console'][type].apply([
    '%c[$level] %c$name${message.contains('\n') ? ':\n' : ': '}%c$message',
    'color: #9E9E9E',
    'color: #${randomColor(name)}',
    ''
  ], thisArg: context['console']);

This fails with:

  at Object.throw_ [as throw] (http://localhost:60993/dart_sdk.js:4322:11)
    at js.JsObject._fromJs.[dartx._get] (http://localhost:60993/dart_sdk.js:102067:70)
    at Object._checkAndCall (http://localhost:60993/dart_sdk.js:4513:16)
    at Object.callMethod (http://localhost:60993/dart_sdk.js:4575:17)
    at Object.dsend (http://localhost:60993/dart_sdk.js:4578:17)
    at Object._printToConsole
...

Interesting is that this doesn't happen during the first calls to _printToConsole. There seems to be some other code in my project that has a side effect and makes later calls fail. The prototype looks strange, too:
image

Even if I replace it with a simple print I run into another issue, a NoSuchMethodError for a case were I know that the accessed field exists 🤔

@sigmundch
Copy link
Member

@Fox32 sorry to hear you are seeing additional failures in Chrome 87.

Do you know if this only relates to code that uses dart:js? On a related note - we normally recommend using package:js instead of dart:js, it tends to have much lower performance overhead.

Let us know if you find more details as you investigate, and please feel free to open a new bug if you have some simple steps that we could use to reproduce the problem locally.

@sigmundch
Copy link
Member

@Fox32 the GamepadList in the prototype chain was a good clue. I'm not sure if this is the only issue, but it does appear that the APIs around that changed in Chrome 86. I filed #43750 to track that.

@Fox32
Copy link

Fox32 commented Oct 11, 2020

@sigmundch The GamepadList seems to be the root cause in our application. If I resolve the issue, everything seems to work fine again. We have a workaround in place for #31017 that broke too, but that is not a new issue. I had to change that workaround a bit.

On a related note - we normally recommend using package:js instead of dart:js

Yeah we have some code from the time before you recommended it. Looks like it's time to migrate them to package:js. Just did that and it was pretty easy 🤔 I remembered that my last tries on package:js were quite painful...

Thank you for taking a look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. P1 A high priority bug; for example, a single project is unusable or has many test failures web-dev-compiler
Projects
None yet
Development

No branches or pull requests

6 participants