Use ReceivedBadMessage throughout guest_view code |
||||||
Issue descriptionIn components/guest_view/browser/ and parts of extensions/browser/guest_view/, we kill misbehaving renderers without using ReceivedBadMessage. Instead we directly shutdown the renderer process and record our own BadMessageTerminate_BPGM user action. We should use ReceivedBadMessage helpers so that we have consistent logging, metrics recording, and process termination.
,
Aug 4 2017
,
Aug 9 2017
,
Aug 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/33fc0784cd6711bca5014853c84ea7567d7c7c31 commit 33fc0784cd6711bca5014853c84ea7567d7c7c31 Author: Kevin McNee <mcnee@chromium.org> Date: Fri Aug 11 14:07:17 2017 Use ReceivedBadMessage throughout guest_view code. Some parts of guest_view are still using ad-hoc code for killing misbehaving renderers where we directly shutdown the renderer process and record our own BadMessageTerminate_BPGM user action. We now use ReceivedBadMessage helpers to have consistent logging, metrics recording, and process termination. This introduces a new histogram Stability.BadMessageTerminated.GuestView along with its enum BadMessageReasonGuestView. This also deprecates BadMessageTerminate_BPGM. Bug: 752596 Change-Id: I5b58e45b069fdc71c60f9e904aca86f7fb529c1c Reviewed-on: https://chromium-review.googlesource.com/606868 Commit-Queue: Kevin McNee <mcnee@chromium.org> Reviewed-by: Jesse Doherty <jwd@chromium.org> Reviewed-by: Lucas Gadani <lfg@chromium.org> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org> Cr-Commit-Position: refs/heads/master@{#493722} [modify] https://crrev.com/33fc0784cd6711bca5014853c84ea7567d7c7c31/components/guest_view/browser/BUILD.gn [add] https://crrev.com/33fc0784cd6711bca5014853c84ea7567d7c7c31/components/guest_view/browser/PRESUBMIT.py [add] https://crrev.com/33fc0784cd6711bca5014853c84ea7567d7c7c31/components/guest_view/browser/bad_message.cc [add] https://crrev.com/33fc0784cd6711bca5014853c84ea7567d7c7c31/components/guest_view/browser/bad_message.h [modify] https://crrev.com/33fc0784cd6711bca5014853c84ea7567d7c7c31/components/guest_view/browser/guest_view_manager.cc [modify] https://crrev.com/33fc0784cd6711bca5014853c84ea7567d7c7c31/extensions/browser/bad_message.h [modify] https://crrev.com/33fc0784cd6711bca5014853c84ea7567d7c7c31/extensions/browser/guest_view/web_view/web_view_guest.cc [modify] https://crrev.com/33fc0784cd6711bca5014853c84ea7567d7c7c31/tools/metrics/actions/actions.xml [modify] https://crrev.com/33fc0784cd6711bca5014853c84ea7567d7c7c31/tools/metrics/histograms/enums.xml [modify] https://crrev.com/33fc0784cd6711bca5014853c84ea7567d7c7c31/tools/metrics/histograms/histograms.xml [modify] https://crrev.com/33fc0784cd6711bca5014853c84ea7567d7c7c31/tools/metrics/histograms/update_bad_message_reasons.py
,
Aug 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e7cfa826feb7cd761d348604b0e08b7ca0c162b9 commit e7cfa826feb7cd761d348604b0e08b7ca0c162b9 Author: Kevin McNee <mcnee@chromium.org> Date: Wed Aug 16 18:01:27 2017 Kill renderer on unexpected message in OnAttachGuest. If we receive a message from the renderer to GuestViewMessageFilter::OnAttachGuest before the creation of a GuestViewManager, then the renderer is misbehaving. The message was just being dropped. We now kill the renderer. Bug: 752596 Change-Id: I8a46dd658ce36b251958f272bd36fe091e70b82d Reviewed-on: https://chromium-review.googlesource.com/617164 Reviewed-by: Lucas Gadani <lfg@chromium.org> Commit-Queue: Kevin McNee <mcnee@chromium.org> Cr-Commit-Position: refs/heads/master@{#494856} [modify] https://crrev.com/e7cfa826feb7cd761d348604b0e08b7ca0c162b9/components/guest_view/browser/guest_view_message_filter.cc
,
Aug 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/144bfa703e937fd123986080b80bd671ee49e87b commit 144bfa703e937fd123986080b80bd671ee49e87b Author: Kevin McNee <mcnee@chromium.org> Date: Mon Aug 21 16:54:09 2017 Don't send the browser a message to resize MHVGs before their creation. When MimeHandlerViewContainer::DidResizeElement receives a geometry update, it sends ExtensionsGuestViewHostMsg_ResizeGuest to the browser. However, the first update happens before MimeHandlerViewContainer messages the browser to create a MimeHandlerViewGuest. ExtensionsGuestViewMessageFilter::OnResizeGuest will drop the resize message. Moreover, OnResizeGuest considers this to be misbehaviour on the part of the renderer, although it is not currently enforced. We now wait for the browser to acknowledge the creation of the MimeHandlerViewGuest before we send resize messages upon geometry updates. Bug: 752596 Change-Id: I2a15da5a0ba396946b687893bd54d7fec0727aa8 Reviewed-on: https://chromium-review.googlesource.com/619449 Commit-Queue: Kevin McNee <mcnee@chromium.org> Reviewed-by: Ehsan Karamad <ekaramad@chromium.org> Reviewed-by: Lucas Gadani <lfg@chromium.org> Cr-Commit-Position: refs/heads/master@{#495965} [modify] https://crrev.com/144bfa703e937fd123986080b80bd671ee49e87b/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc [modify] https://crrev.com/144bfa703e937fd123986080b80bd671ee49e87b/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.h
,
Aug 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0a29d1d5cd3c4126a190084ffec6b304718f64db commit 0a29d1d5cd3c4126a190084ffec6b304718f64db Author: Kevin McNee <mcnee@chromium.org> Date: Mon Aug 21 20:26:07 2017 Kill renderer on unexpected message in OnResizeGuest. If we receive a message from the renderer to ExtensionsGuestViewMessageFilter::OnResizeGuest before the creation of a GuestViewManager, then the renderer is misbehaving. The message was just being dropped. We now kill the renderer. Bug: 752596 Change-Id: I216e93ae8eef7bcfce7f5aa4be718dcf758a4496 Reviewed-on: https://chromium-review.googlesource.com/617065 Reviewed-by: Lucas Gadani <lfg@chromium.org> Commit-Queue: Kevin McNee <mcnee@chromium.org> Cr-Commit-Position: refs/heads/master@{#496047} [modify] https://crrev.com/0a29d1d5cd3c4126a190084ffec6b304718f64db/extensions/browser/guest_view/extensions_guest_view_message_filter.cc
,
Aug 21 2017
,
Aug 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1739dce5f98f2d381caaa024f3586b8e1120b01c commit 1739dce5f98f2d381caaa024f3586b8e1120b01c Author: Kevin McNee <mcnee@chromium.org> Date: Wed Aug 30 14:40:28 2017 Revert OnResizeGuest message order enforcement due to dropped resize. This reverts the dropping of resize messages before the ack of guest creation. Since this reintroduces the behaviour for which we kill the renderer, it is also necessary to revert the killing of the renderer so that the former can be safely reverted. If the first resize seen by MimeHandlerViewContainer::DidResizeElement is between when the creation message is sent and when the ack is received, the resize will not be sent to the browser. I'm reverting the enforcement for now in order to correct this on ToT and will follow up with corrected logic so we can start enforcing again. Bug: 759895, 752596 Change-Id: I5044b996ae25d196a43f07f41240f5cbb654dee1 Reviewed-on: https://chromium-review.googlesource.com/641337 Commit-Queue: Kevin McNee <mcnee@chromium.org> Reviewed-by: Ehsan Karamad <ekaramad@chromium.org> Cr-Commit-Position: refs/heads/master@{#498458} [modify] https://crrev.com/1739dce5f98f2d381caaa024f3586b8e1120b01c/extensions/browser/guest_view/extensions_guest_view_message_filter.cc [modify] https://crrev.com/1739dce5f98f2d381caaa024f3586b8e1120b01c/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc [modify] https://crrev.com/1739dce5f98f2d381caaa024f3586b8e1120b01c/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.h
,
Aug 30 2017
There was an issue with resizing messages being dropped (759895), the CLs related to OnResizeGuest were reverted since we're so close to the branch point. A proper fix that allows us to keep the enforcement will follow.
,
Sep 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bb41e586f29a75f4fa298f512566da3c1c431d64 commit bb41e586f29a75f4fa298f512566da3c1c431d64 Author: Kevin McNee <mcnee@chromium.org> Date: Thu Sep 07 21:37:56 2017 Re-enable renderer killing on unexpected message in OnResizeGuest. We reverted the dropping of resize messages before the ack of guest creation due to the first resize seen by MimeHandlerViewContainer::DidResizeElement getting dropped if it is between when the creation message is sent and when the ack is received. We also reverted the enforcement in OnResizeGuest to make the above revert safe. We now only drop resize messages before the creation message, as the creation message contains the necessary size information and sending a resize message after the creation message is well-behaved. This allows us to re-enable the enforcement. Bug: 752596 , 759895 Change-Id: I0f6da2fe9a72cb9fa350ccda5f1db28780099725 Reviewed-on: https://chromium-review.googlesource.com/652769 Reviewed-by: Ehsan Karamad <ekaramad@chromium.org> Commit-Queue: Kevin McNee <mcnee@chromium.org> Cr-Commit-Position: refs/heads/master@{#500396} [modify] https://crrev.com/bb41e586f29a75f4fa298f512566da3c1c431d64/extensions/browser/guest_view/extensions_guest_view_message_filter.cc [modify] https://crrev.com/bb41e586f29a75f4fa298f512566da3c1c431d64/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc
,
Sep 7 2017
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by mcnee@chromium.org
, Aug 4 2017