Have WebContents deletion work asynchronously for navigation in Clank |
|||
Issue descriptionQuoted from clamy@'s note: "right now it's asynchronous on all platforms, except Clank. Because of that, we've had to make some specific code in the main path of navigation asynchronous just on Clank, because we were risking UAFs. It'd be better if we didn't risk UAFs when executing a method from inside content/."
,
Jun 13 2017
Other cases that will close the current WebContents are: - Launching app from Incognito tab - Download files - Local file access permission (in InterceptNavigationDelegateImpl -> ExternalNavigationHandler -> ExternalNavigationDelegateImpl) These are all safe from UAF concern because they all close the tab after user interaction (warning dialog about leaving incognito mode, download accept/cancel, etc), effectively doing it behind the UI message loop. So I think we're covered in all grounds.
,
Jun 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d5545cdaf40989792dd8cc2483304c613ac9f54c commit d5545cdaf40989792dd8cc2483304c613ac9f54c Author: Jinsuk Kim <jinsukkim@chromium.org> Date: Sat Jun 17 15:55:52 2017 PlzNavigate: Run InterceptnavigationThrottle in synchronous mode Puts InterceptNavigationThrottle's shouldIgnoreNavigation callback in synchronous mode, and defer the destruction of WebContents (and the associated tab) while navigation throttle is in progress and finishes accessing the object. This makes the navigation throttle flow simpler on Android. BUG= 732260 Change-Id: Ib151b5458a3adde927db22c825f708c5266f6372 Reviewed-on: https://chromium-review.googlesource.com/531144 Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org> Reviewed-by: Camille Lamy <clamy@chromium.org> Reviewed-by: Bernhard Bauer <bauerb@chromium.org> Reviewed-by: Selim Gurun <sgurun@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org> Reviewed-by: Ted Choc <tedchoc@chromium.org> Cr-Commit-Position: refs/heads/master@{#480304} [modify] https://crrev.com/d5545cdaf40989792dd8cc2483304c613ac9f54c/chrome/android/java/src/org/chromium/chrome/browser/tab/InterceptNavigationDelegateImpl.java [modify] https://crrev.com/d5545cdaf40989792dd8cc2483304c613ac9f54c/chrome/browser/apps/app_url_redirector.cc [modify] https://crrev.com/d5545cdaf40989792dd8cc2483304c613ac9f54c/chrome/browser/plugins/flash_download_interception.cc [modify] https://crrev.com/d5545cdaf40989792dd8cc2483304c613ac9f54c/components/navigation_interception/intercept_navigation_delegate.cc [modify] https://crrev.com/d5545cdaf40989792dd8cc2483304c613ac9f54c/components/navigation_interception/intercept_navigation_throttle.cc [modify] https://crrev.com/d5545cdaf40989792dd8cc2483304c613ac9f54c/components/navigation_interception/intercept_navigation_throttle.h [modify] https://crrev.com/d5545cdaf40989792dd8cc2483304c613ac9f54c/components/navigation_interception/intercept_navigation_throttle_unittest.cc
,
Jul 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c644aad97a1e9efa2c818099fd33cee6386a09bf commit c644aad97a1e9efa2c818099fd33cee6386a09bf Author: Jinsuk Kim <jinsukkim@chromium.org> Date: Mon Jul 03 21:32:44 2017 Reland "PlzNavigate: Run InterceptnavigationThrottle in synchronous mode"" The reverted CL ran the tab closing task in InterceptNavigation- Throttle in an asynchronous fashion (posting task onto UI thread again) to avoid UAF. In addition to that, loading a new URL on the current tab by the throttle should have been given the same treat because it cancels the ongoing navigation as well, hence can cause UAF error. This CL handles the missing case. BUG= 732260 This reverts commit 6d4d2da0bda314416b87c1c3dde87ac96e9634f4. Change-Id: I423d09c30f01fcd1d1f6ddf806d0a0123609bf9a Reviewed-on: https://chromium-review.googlesource.com/552143 Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org> Reviewed-by: Bernhard Bauer <bauerb@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Reviewed-by: Selim Gurun <sgurun@chromium.org> Reviewed-by: Camille Lamy <clamy@chromium.org> Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org> Cr-Commit-Position: refs/heads/master@{#484030} [modify] https://crrev.com/c644aad97a1e9efa2c818099fd33cee6386a09bf/chrome/android/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImpl.java [modify] https://crrev.com/c644aad97a1e9efa2c818099fd33cee6386a09bf/chrome/android/java/src/org/chromium/chrome/browser/tab/InterceptNavigationDelegateImpl.java [modify] https://crrev.com/c644aad97a1e9efa2c818099fd33cee6386a09bf/chrome/browser/apps/app_url_redirector.cc [modify] https://crrev.com/c644aad97a1e9efa2c818099fd33cee6386a09bf/chrome/browser/plugins/flash_download_interception.cc [modify] https://crrev.com/c644aad97a1e9efa2c818099fd33cee6386a09bf/components/navigation_interception/intercept_navigation_delegate.cc [modify] https://crrev.com/c644aad97a1e9efa2c818099fd33cee6386a09bf/components/navigation_interception/intercept_navigation_throttle.cc [modify] https://crrev.com/c644aad97a1e9efa2c818099fd33cee6386a09bf/components/navigation_interception/intercept_navigation_throttle.h [modify] https://crrev.com/c644aad97a1e9efa2c818099fd33cee6386a09bf/components/navigation_interception/intercept_navigation_throttle_unittest.cc
,
Jul 4 2017
,
Jul 21 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by jinsuk...@chromium.org
, Jun 12 2017