fix(core): fix #20582, don't need to wrap zone in location change listener

[JiaLiPassion]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: 20582

What is the new behavior?

remove Zone.current.wrap in router.ts when setUpLocationChangeListener, because
zone.js already patch onPopState, so location change listener will always run in ngZone.
https://github.com/angular/angular/blob/master/packages/common/src/location/location.ts#L55

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

[mhevery]

I would much rather remove the reference to Zone 25e5b2f#diff-7f0963cbc503a361ded7d2bfec5f01b9R386 rather than trying to create a mock. Could we do that instead?

[JiaLiPassion]

@mhevery , sure, I will try to rewrite it.
And I will also check the other code, such as animation_renderer to make sure all Zone.current be handled correctly under NoopNgZone.

[JiaLiPassion]

@mhevery , I have checked the source, and I think because zone.js already patch popstate, so we don't need to use Zone.current.wrap in router any longer,
here is the issue that why use Zone.current.wrap, #10096, I have created a sample application to test, without Zone.current.wrap, in safari/IE11, the back button will work as normal, and the code of ngOnit() will be called in ngZone.

Please review. thank you.

[mohammedzamakhan]

Any updates on this PR??

[ngbot]

Hi @JiaLiPassion! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

[ngbot]

Hi @JiaLiPassion! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

[mhevery]

@JiaLiPassion could you rebase this on master so that we can get this in.

[JiaLiPassion]

@mhevery , I have rebased the PR, please review, thank you!

[mhevery]

http://test/OCL:183899528:BASE:184015406:1517424943121:4fd0de04 FAIL
http://test/OCL:183899528:BASE:184088093:1517461446680:15ec109 FAIL wrong rebase
http://test/OCL:183899528:BASE:184090890:1517464368901:6e9301bf
http://test/OCL:183899528:BASE:184091074:1517464576226:4d6d4aa5
http://test/OCL:183899528:BASE:184146508:1517504458180:391951fc

[alxhub]

Greetings from your friendly neighborhood Angular caretaker, @jasonaden @JiaLiPassion,

This is targeted to both master and the patch branch, but doesn't cleanly apply on the patch branch. Can you please split the PR into two, one against master and the other against the patch branch?

Thanks,
Alex

[JiaLiPassion]

@alxhub , thank you for review, I am not sure how to do to split the PR to different branch, could you tell me more?

[alxhub]

@JiaLiPassion sure thing! When you create a branch locally, you typically start on master. To create one for the patch branch, you'll want to start on 5.2.x instead, then in Github when you make the PR, that becomes your target branch.

[mhevery]

I have changed the target to master only. @JiaLiPassion Please create an we PR and rebase it on top of 5.2.x branch and resubmit.

[JiaLiPassion]

@mhevery , @alxhub, thank you! I have created a PR which target 5.2.x, #22007. please review.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}