Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2687943002:
Turn off SiteIsolationPolicy::AreCrossProcessFramesPossible for Android. (Closed)
DescriptionTurn off SiteIsolationPolicy::AreCrossProcessFramesPossible for Android.
AreCrossProcessFramesPossible was turned on by default for all
platforms in https://codereview.chromium.org/2676823003, but that
shouldn't have included Android, which still doesn't have input event
routing fully implemented. Additionally, turning it on revealed a
performance regression on Android that will need to be investigated.
BUG=690229, 545200
Review-Url: https://codereview.chromium.org/2687943002
Cr-Commit-Position: refs/heads/master@{#449491}
Committed: https://chromium.googlesource.com/chromium/src/+/a4c553be55b1261fd0beda3ea8b527b787ab50a1
Patch Set 1 #
Total comments: 2
Patch Set 2 : Use the old policy for Android #Patch Set 3 : Fix Android builds (bring back the necessary headers) #Messages
Total messages: 27 (19 generated)
The CQ bit was checked by alexmos@chromium.org to run a CQ dry run
alexmos@chromium.org changed reviewers: + nasko@chromium.org
Nasko, can you please take a look?
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
https://codereview.chromium.org/2687943002/diff/1/content/common/site_isolati... File content/common/site_isolation_policy.cc (right): https://codereview.chromium.org/2687943002/diff/1/content/common/site_isolati... content/common/site_isolation_policy.cc:18: return false; Shouldn't this be the old code that we removed? It should still be conditional on whether we run in --site-per-process.
The CQ bit was checked by alexmos@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Turn off SiteIsolationPolicy::AreCrossProcessFramesPossible for Android. BUG=690229 ========== to ========== Turn off SiteIsolationPolicy::AreCrossProcessFramesPossible for Android. AreCrossProcessFramesPossible was turned on by default for all platforms in https://codereview.chromium.org/2676823003, but that shouldn't have included Android, which still doesn't have input event routing fully implemented. Additionally, turning it on revealed a performance regression on Android that will need to be investigated. BUG=690229, 545200 ==========
https://codereview.chromium.org/2687943002/diff/1/content/common/site_isolati... File content/common/site_isolation_policy.cc (right): https://codereview.chromium.org/2687943002/diff/1/content/common/site_isolati... content/common/site_isolation_policy.cc:18: return false; On 2017/02/09 21:05:25, nasko (very slow) wrote: > Shouldn't this be the old code that we removed? It should still be conditional > on whether we run in --site-per-process. Indeed! Thanks for catching, and I'm glad the bots caught it too. Fixed now.
LGTM
The CQ bit was unchecked by alexmos@chromium.org
The CQ bit was checked by alexmos@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
The CQ bit was checked by alexmos@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by alexmos@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nasko@chromium.org Link to the patchset: https://codereview.chromium.org/2687943002/#ps40001 (title: "Fix Android builds (bring back the necessary headers)")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486686738911220,
"parent_rev": "0471cce8b9981ab78b2ab8cd11ae371611b0ab12", "commit_rev":
"a4c553be55b1261fd0beda3ea8b527b787ab50a1"}
Message was sent while issue was closed.
Description was changed from ========== Turn off SiteIsolationPolicy::AreCrossProcessFramesPossible for Android. AreCrossProcessFramesPossible was turned on by default for all platforms in https://codereview.chromium.org/2676823003, but that shouldn't have included Android, which still doesn't have input event routing fully implemented. Additionally, turning it on revealed a performance regression on Android that will need to be investigated. BUG=690229, 545200 ========== to ========== Turn off SiteIsolationPolicy::AreCrossProcessFramesPossible for Android. AreCrossProcessFramesPossible was turned on by default for all platforms in https://codereview.chromium.org/2676823003, but that shouldn't have included Android, which still doesn't have input event routing fully implemented. Additionally, turning it on revealed a performance regression on Android that will need to be investigated. BUG=690229, 545200 Review-Url: https://codereview.chromium.org/2687943002 Cr-Commit-Position: refs/heads/master@{#449491} Committed: https://chromium.googlesource.com/chromium/src/+/a4c553be55b1261fd0beda3ea8b5... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/a4c553be55b1261fd0beda3ea8b5... |
