SiteProcessMap::RegisterProcess can overwrite existing entries |
||
Issue descriptionThe current implementation of SiteProcessMap::RegisterProcess does not check if the site being registered is already present in the map. This can allow processes in some cases to overwrite the entry in the table and become the process hosting the site in the process-per-site process model.
,
Mar 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0e9d619f88ffabc3c237cbdf75603df8af9925e6 commit 0e9d619f88ffabc3c237cbdf75603df8af9925e6 Author: nasko <nasko@chromium.org> Date: Wed Mar 15 00:42:58 2017 Check for already existing entry when adding to SiteProcessMap. The SiteProcessMap::RegisterProcess can overwrite existing entries in the map, which can lead to incorrect process being mapped to a site in process-per-site mode. This CL checks for existence and avoids the overwrite. BUG= 700514 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2739193004 Cr-Commit-Position: refs/heads/master@{#456920} [modify] https://crrev.com/0e9d619f88ffabc3c237cbdf75603df8af9925e6/chrome/browser/extensions/process_management_browsertest.cc [add] https://crrev.com/0e9d619f88ffabc3c237cbdf75603df8af9925e6/chrome/test/data/extensions/web_request_site_process_registration/blocked.html [add] https://crrev.com/0e9d619f88ffabc3c237cbdf75603df8af9925e6/chrome/test/data/extensions/web_request_site_process_registration/manifest.json [add] https://crrev.com/0e9d619f88ffabc3c237cbdf75603df8af9925e6/chrome/test/data/extensions/web_request_site_process_registration/test.html [add] https://crrev.com/0e9d619f88ffabc3c237cbdf75603df8af9925e6/chrome/test/data/extensions/web_request_site_process_registration/test.js [modify] https://crrev.com/0e9d619f88ffabc3c237cbdf75603df8af9925e6/content/browser/frame_host/navigator_impl.cc [modify] https://crrev.com/0e9d619f88ffabc3c237cbdf75603df8af9925e6/content/browser/renderer_host/render_process_host_impl.cc
,
Mar 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/96bb282f55edfa739e4e5f6633bb5fce61583e87 commit 96bb282f55edfa739e4e5f6633bb5fce61583e87 Author: tzik <tzik@chromium.org> Date: Wed Mar 15 06:19:46 2017 Revert of Check for already existing entry when adding to SiteProcessMap. (patchset #7 id:160001 of https://codereview.chromium.org/2739193004/ ) Reason for revert: Speculative revert to see a bot failure. SubresourceFilterBrowserTest.FailedProvisionalLoadInMainframe got flaky around this CL, and this is relatively more suspicious in the failing range. BUG= 701663 Original issue's description: > Check for already existing entry when adding to SiteProcessMap. > > The SiteProcessMap::RegisterProcess can overwrite existing entries > in the map, which can lead to incorrect process being mapped to > a site in process-per-site mode. This CL checks for existence and > avoids the overwrite. > > BUG= 700514 > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation > > Review-Url: https://codereview.chromium.org/2739193004 > Cr-Commit-Position: refs/heads/master@{#456920} > Committed: https://chromium.googlesource.com/chromium/src/+/0e9d619f88ffabc3c237cbdf75603df8af9925e6 TBR=creis@chromium.org,rdevlin.cronin@chromium.org,nasko@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 700514 Review-Url: https://codereview.chromium.org/2749543004 Cr-Commit-Position: refs/heads/master@{#457010} [modify] https://crrev.com/96bb282f55edfa739e4e5f6633bb5fce61583e87/chrome/browser/extensions/process_management_browsertest.cc [delete] https://crrev.com/f9d855c6039ca2daddbd4c3d08caac142fd15966/chrome/test/data/extensions/web_request_site_process_registration/blocked.html [delete] https://crrev.com/f9d855c6039ca2daddbd4c3d08caac142fd15966/chrome/test/data/extensions/web_request_site_process_registration/manifest.json [delete] https://crrev.com/f9d855c6039ca2daddbd4c3d08caac142fd15966/chrome/test/data/extensions/web_request_site_process_registration/test.html [delete] https://crrev.com/f9d855c6039ca2daddbd4c3d08caac142fd15966/chrome/test/data/extensions/web_request_site_process_registration/test.js [modify] https://crrev.com/96bb282f55edfa739e4e5f6633bb5fce61583e87/content/browser/frame_host/navigator_impl.cc [modify] https://crrev.com/96bb282f55edfa739e4e5f6633bb5fce61583e87/content/browser/renderer_host/render_process_host_impl.cc
,
Mar 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0bcbed7d67f9a2813e1a278aada6c047a26bbbff commit 0bcbed7d67f9a2813e1a278aada6c047a26bbbff Author: nasko <nasko@chromium.org> Date: Wed Mar 15 17:03:35 2017 Reland of Check for already existing entry when adding to SiteProcessMap. (patchset #1 id:1 of https://codereview.chromium.org/2749543004/ ) Reason for revert: Reverting the revert to see if this was indeed the culprit for the flakiness. Original issue's description: > Revert of Check for already existing entry when adding to SiteProcessMap. (patchset #7 id:160001 of https://codereview.chromium.org/2739193004/ ) > > Reason for revert: > Speculative revert to see a bot failure. > > SubresourceFilterBrowserTest.FailedProvisionalLoadInMainframe got flaky around this CL, and this is relatively more suspicious in the failing range. > > BUG= 701663 > > > Original issue's description: > > Check for already existing entry when adding to SiteProcessMap. > > > > The SiteProcessMap::RegisterProcess can overwrite existing entries > > in the map, which can lead to incorrect process being mapped to > > a site in process-per-site mode. This CL checks for existence and > > avoids the overwrite. > > > > BUG= 700514 > > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation > > > > Review-Url: https://codereview.chromium.org/2739193004 > > Cr-Commit-Position: refs/heads/master@{#456920} > > Committed: https://chromium.googlesource.com/chromium/src/+/0e9d619f88ffabc3c237cbdf75603df8af9925e6 > > TBR=creis@chromium.org,rdevlin.cronin@chromium.org,nasko@chromium.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG= 700514 > > Review-Url: https://codereview.chromium.org/2749543004 > Cr-Commit-Position: refs/heads/master@{#457010} > Committed: https://chromium.googlesource.com/chromium/src/+/96bb282f55edfa739e4e5f6633bb5fce61583e87 TBR=creis@chromium.org,rdevlin.cronin@chromium.org,tzik@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 701663 Review-Url: https://codereview.chromium.org/2755643003 Cr-Commit-Position: refs/heads/master@{#457110} [modify] https://crrev.com/0bcbed7d67f9a2813e1a278aada6c047a26bbbff/chrome/browser/extensions/process_management_browsertest.cc [add] https://crrev.com/0bcbed7d67f9a2813e1a278aada6c047a26bbbff/chrome/test/data/extensions/web_request_site_process_registration/blocked.html [add] https://crrev.com/0bcbed7d67f9a2813e1a278aada6c047a26bbbff/chrome/test/data/extensions/web_request_site_process_registration/manifest.json [add] https://crrev.com/0bcbed7d67f9a2813e1a278aada6c047a26bbbff/chrome/test/data/extensions/web_request_site_process_registration/test.html [add] https://crrev.com/0bcbed7d67f9a2813e1a278aada6c047a26bbbff/chrome/test/data/extensions/web_request_site_process_registration/test.js [modify] https://crrev.com/0bcbed7d67f9a2813e1a278aada6c047a26bbbff/content/browser/frame_host/navigator_impl.cc [modify] https://crrev.com/0bcbed7d67f9a2813e1a278aada6c047a26bbbff/content/browser/renderer_host/render_process_host_impl.cc
,
Mar 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7542ac912821eb3d5c939f17de433621d60f7fab commit 7542ac912821eb3d5c939f17de433621d60f7fab Author: nasko <nasko@chromium.org> Date: Wed Mar 15 21:20:00 2017 Revert of Check for already existing entry when adding to SiteProcessMap. (patchset #1 id:1 of https://codereview.chromium.org/2755643003/ ) Reason for revert: Test is flaking again, reverting the reland. Original issue's description: > Reland of Check for already existing entry when adding to SiteProcessMap. (patchset #1 id:1 of https://codereview.chromium.org/2749543004/ ) > > Reason for revert: > Reverting the revert to see if this was indeed the culprit for the flakiness. > > Original issue's description: > > Revert of Check for already existing entry when adding to SiteProcessMap. (patchset #7 id:160001 of https://codereview.chromium.org/2739193004/ ) > > > > Reason for revert: > > Speculative revert to see a bot failure. > > > > SubresourceFilterBrowserTest.FailedProvisionalLoadInMainframe got flaky around this CL, and this is relatively more suspicious in the failing range. > > > > BUG= 701663 > > > > > > Original issue's description: > > > Check for already existing entry when adding to SiteProcessMap. > > > > > > The SiteProcessMap::RegisterProcess can overwrite existing entries > > > in the map, which can lead to incorrect process being mapped to > > > a site in process-per-site mode. This CL checks for existence and > > > avoids the overwrite. > > > > > > BUG= 700514 > > > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation > > > > > > Review-Url: https://codereview.chromium.org/2739193004 > > > Cr-Commit-Position: refs/heads/master@{#456920} > > > Committed: https://chromium.googlesource.com/chromium/src/+/0e9d619f88ffabc3c237cbdf75603df8af9925e6 > > > > TBR=creis@chromium.org,rdevlin.cronin@chromium.org,nasko@chromium.org > > # Skipping CQ checks because original CL landed less than 1 days ago. > > NOPRESUBMIT=true > > NOTREECHECKS=true > > NOTRY=true > > BUG= 700514 > > > > Review-Url: https://codereview.chromium.org/2749543004 > > Cr-Commit-Position: refs/heads/master@{#457010} > > Committed: https://chromium.googlesource.com/chromium/src/+/96bb282f55edfa739e4e5f6633bb5fce61583e87 > > TBR=creis@chromium.org,rdevlin.cronin@chromium.org,tzik@chromium.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG= 701663 > > Review-Url: https://codereview.chromium.org/2755643003 > Cr-Commit-Position: refs/heads/master@{#457110} > Committed: https://chromium.googlesource.com/chromium/src/+/0bcbed7d67f9a2813e1a278aada6c047a26bbbff TBR=creis@chromium.org,rdevlin.cronin@chromium.org,tzik@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 701663 Review-Url: https://codereview.chromium.org/2756573002 Cr-Commit-Position: refs/heads/master@{#457211} [modify] https://crrev.com/7542ac912821eb3d5c939f17de433621d60f7fab/chrome/browser/extensions/process_management_browsertest.cc [delete] https://crrev.com/3e36a03e908bfc7bf9a4335611ae4789280c1506/chrome/test/data/extensions/web_request_site_process_registration/blocked.html [delete] https://crrev.com/3e36a03e908bfc7bf9a4335611ae4789280c1506/chrome/test/data/extensions/web_request_site_process_registration/manifest.json [delete] https://crrev.com/3e36a03e908bfc7bf9a4335611ae4789280c1506/chrome/test/data/extensions/web_request_site_process_registration/test.html [delete] https://crrev.com/3e36a03e908bfc7bf9a4335611ae4789280c1506/chrome/test/data/extensions/web_request_site_process_registration/test.js [modify] https://crrev.com/7542ac912821eb3d5c939f17de433621d60f7fab/content/browser/frame_host/navigator_impl.cc [modify] https://crrev.com/7542ac912821eb3d5c939f17de433621d60f7fab/content/browser/renderer_host/render_process_host_impl.cc
,
Mar 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0485e8ec0ab7d527744dcd61328b7f23a1eecfda commit 0485e8ec0ab7d527744dcd61328b7f23a1eecfda Author: engedy <engedy@chromium.org> Date: Fri Mar 17 16:48:58 2017 Make SubresourceFilterBrowserTest.FailedProvisionalLoadInMainframe non-flaky. When a provisional load fails with no server-supplied error page, Chrome's own navigation error page is shown. This also triggers a background request to load navigation corrections (aka. Link Doctor). Once the results are back, there is a navigation to a second error page with the suggestions. Thes CL makes the SubresourceFilterBrowserTest.FailedProvisionalLoadInMainframe expect and wait for this second navigation. Otherwise the completion of loading the Link Doctor error page would be at race with that of subsequent navigations initiated in the test, leading to flakiness. BUG=637415, 700514 Review-Url: https://codereview.chromium.org/2754173003 Cr-Commit-Position: refs/heads/master@{#457796} [modify] https://crrev.com/0485e8ec0ab7d527744dcd61328b7f23a1eecfda/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
,
Mar 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d1aaa76016c981807fbe10c53c00de6061200c98 commit d1aaa76016c981807fbe10c53c00de6061200c98 Author: nasko <nasko@chromium.org> Date: Sat Mar 18 02:14:45 2017 Check for already existing entry when adding to SiteProcessMap. The SiteProcessMap::RegisterProcess can overwrite existing entries in the map, which can lead to incorrect process being mapped to a site in process-per-site mode. This CL checks for existence and avoids the overwrite. This CL is a reland of https://codereview.chromium.org/2739193004/ after the flaky test was fixed in https://codereview.chromium.org/2754173003/ BUG= 700514 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2760733002 Cr-Commit-Position: refs/heads/master@{#457925} [modify] https://crrev.com/d1aaa76016c981807fbe10c53c00de6061200c98/chrome/browser/extensions/process_management_browsertest.cc [modify] https://crrev.com/d1aaa76016c981807fbe10c53c00de6061200c98/chrome/test/data/extensions/api_test/webrequest/test_blocking.js [add] https://crrev.com/d1aaa76016c981807fbe10c53c00de6061200c98/chrome/test/data/extensions/web_request_site_process_registration/blocked.html [add] https://crrev.com/d1aaa76016c981807fbe10c53c00de6061200c98/chrome/test/data/extensions/web_request_site_process_registration/manifest.json [add] https://crrev.com/d1aaa76016c981807fbe10c53c00de6061200c98/chrome/test/data/extensions/web_request_site_process_registration/test.html [add] https://crrev.com/d1aaa76016c981807fbe10c53c00de6061200c98/chrome/test/data/extensions/web_request_site_process_registration/test.js [modify] https://crrev.com/d1aaa76016c981807fbe10c53c00de6061200c98/content/browser/frame_host/navigator_impl.cc [modify] https://crrev.com/d1aaa76016c981807fbe10c53c00de6061200c98/content/browser/renderer_host/render_process_host_impl.cc
,
Mar 20 2017
It looks like the fix is sticking now, so resolving as fixed. |
||
►
Sign in to add a comment |
||
Comment 1 by nasko@chromium.org
, Mar 11 2017