| 
 | 
 | 
 Chromium Code Reviews
 Chromium Code Reviews Issue 
            1729373003:
    Implement touch events for site-isolation.  (Closed)
    
  
    Issue 
            1729373003:
    Implement touch events for site-isolation.  (Closed) 
  | Created: 4 years, 10 months ago by wjmaclean Modified: 4 years, 9 months ago CC: chromium-reviews, creis+watch_chromium.org, yusukes+watch_chromium.org, mlamouri+watch-blink_chromium.org, shuchen+watch_chromium.org, nasko+codewatch_chromium.org, jam, dcheng, nona+watch_chromium.org, darin-cc_chromium.org, blink-reviews, James Su, site-isolation-reviews_chromium.org Base URL: https://chromium.googlesource.com/chromium/src.git@master Target Ref: refs/pending/heads/master Project: chromium Visibility: Public. | DescriptionImplement touch events for site-isolation.
This CL implements the necessary machinery to plumb touch events into
oopif frames, including oopif-based WebView (BrowserTag).
BUG=581892
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
Committed: https://crrev.com/fab616ad24c87bcd843a77dcbdf7fb299c635cf2
Cr-Commit-Position: refs/heads/master@{#378781}
   Patch Set 1 #Patch Set 2 : Restrict test to Aura for now. #Patch Set 3 : Add missing test file. #Patch Set 4 : Add non-NaN force to touch event. #
      Total comments: 14
      
     Patch Set 5 : Fix non-Aura compile. #Patch Set 6 : Address comments. #
      Total comments: 4
      
     Patch Set 7 : Rebase to r378158. #Patch Set 8 : Address comments. #
      Total comments: 10
      
     Patch Set 9 : Address comments. #Patch Set 10 : Add RenderWidgetHostViewMac::ProcessTouchEvent(). #Patch Set 11 : Fix compile guard for test. #Patch Set 12 : Try different synchronization for hit-testing in test. #Patch Set 13 : Use positioned iframe in test to simplify event targeting. #Dependent Patchsets: Messages
    Total messages: 87 (43 generated)
     
 Description was changed from ========== Implement touch events for site-isolation. This CL implements the necessary machinery to plumb touch events into oopif frames, including oopif-based WebView (BrowserTag). BUG=581892 ========== to ========== Implement touch events for site-isolation. This CL implements the necessary machinery to plumb touch events into oopif frames, including oopif-based WebView (BrowserTag). BUG=581892 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation ========== 
 The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run 
 Description was changed from ========== Implement touch events for site-isolation. This CL implements the necessary machinery to plumb touch events into oopif frames, including oopif-based WebView (BrowserTag). BUG=581892 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation ========== to ========== Implement touch events for site-isolation. This CL implements the necessary machinery to plumb touch events into oopif frames, including oopif-based WebView (BrowserTag). BUG=581892 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation ========== 
 wjmaclean@chromium.org changed reviewers: + kenrb@chromium.org 
 kenrb@ - would you like to take a first look at this? 
 On 2016/02/24 18:11:02, wjmaclean wrote: > kenrb@ - would you like to take a first look at this? BTW, I'm expecting this not to run on non-Aura bots right now ... the test relies on RWHV being a RWHVA for now ... 
 Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/1 
 The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run 
 Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/40001 
 The CQ bit was unchecked by commit-bot@chromium.org 
 Dry run: Try jobs failed on following builders: cast_shell_linux on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) 
 The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run 
 Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/60001 
 The CQ bit was unchecked by commit-bot@chromium.org 
 Dry run: Try jobs failed on following builders: cast_shell_android on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...) chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...) chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...) chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...) linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...) 
 The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run 
 Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/80001 
 Thanks James. Just a couple of thoughts on this. https://codereview.chromium.org/1729373003/diff/60001/content/browser/browser... File content/browser/browser_plugin/browser_plugin_guest.cc (right): https://codereview.chromium.org/1729373003/diff/60001/content/browser/browser... content/browser/browser_plugin/browser_plugin_guest.cc:794: RenderWidgetHostViewBase* rwhv = static_cast<RenderWidgetHostViewGuest*>( I don't understand this change. Can the guest WebContents have an RWHV that isn't RWHVGuest? If so, the cast is still wrong. https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... File content/browser/frame_host/cross_process_frame_connector.cc (right): https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... content/browser/frame_host/cross_process_frame_connector.cc:188: // gesture events :-( We don't need this to handle keyboard events, either. See RenderWidgetHostViewAura::ForwardKeyboardEvent. https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... content/browser/frame_host/cross_process_frame_connector.cc:199: view_->ProcessTouchEvent(*static_cast<const blink::WebTouchEvent*>(event), Is this needed? I think the entire method (OnForwardInputEvent) should be able to go away soon, unless I am missing something. https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... File content/browser/frame_host/cross_process_frame_connector.h (right): https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... content/browser/frame_host/cross_process_frame_connector.h:100: void ForwardProcessAckedTouchEvent(const TouchEventWithLatencyInfo& touch, This warrants a comment. https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... File content/browser/frame_host/render_widget_host_view_child_frame.cc (right): https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... content/browser/frame_host/render_widget_host_view_child_frame.cc:48: GetSurfaceIdNamespace(), this); Should this be replaced by a call to RegisterSurfaceIdNamespace()? https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... content/browser/frame_host/render_widget_host_view_child_frame.cc:213: GetSurfaceIdNamespace()); Replace with a call to UnregisterSurfaceIdNamespace()? https://codereview.chromium.org/1729373003/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp (left): https://codereview.chromium.org/1729373003/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp:321: This probably isn't worth touching Blink, I'm guessing it wasn't deliberate? 
 The CQ bit was unchecked by commit-bot@chromium.org 
 Dry run: Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...) 
 The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run 
 Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/100001 
 kenrb@ - ptal? If it looks ok, could you add appropriate reviewers from content/ ? https://codereview.chromium.org/1729373003/diff/60001/content/browser/browser... File content/browser/browser_plugin/browser_plugin_guest.cc (right): https://codereview.chromium.org/1729373003/diff/60001/content/browser/browser... content/browser/browser_plugin/browser_plugin_guest.cc:794: RenderWidgetHostViewBase* rwhv = static_cast<RenderWidgetHostViewGuest*>( On 2016/02/25 00:58:04, kenrb wrote: > I don't understand this change. Can the guest WebContents have an RWHV that > isn't RWHVGuest? If so, the cast is still wrong. I changed one side of the cast, but not the other - fixed. Here I'm trying to use the weakest cast possible, though we can restrict it to RWHVG if that's considered better. https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... File content/browser/frame_host/cross_process_frame_connector.cc (right): https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... content/browser/frame_host/cross_process_frame_connector.cc:188: // gesture events :-( On 2016/02/25 00:58:04, kenrb wrote: > We don't need this to handle keyboard events, either. See > RenderWidgetHostViewAura::ForwardKeyboardEvent. Done. https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... content/browser/frame_host/cross_process_frame_connector.cc:199: view_->ProcessTouchEvent(*static_cast<const blink::WebTouchEvent*>(event), On 2016/02/25 00:58:04, kenrb wrote: > Is this needed? I think the entire method (OnForwardInputEvent) should be able > to go away soon, unless I am missing something. I added this mostly for completeness, but I'm willing to leave this out on an experimental basis until this entire method goes away. Done. https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... File content/browser/frame_host/cross_process_frame_connector.h (right): https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... content/browser/frame_host/cross_process_frame_connector.h:100: void ForwardProcessAckedTouchEvent(const TouchEventWithLatencyInfo& touch, On 2016/02/25 00:58:04, kenrb wrote: > This warrants a comment. Done. https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... File content/browser/frame_host/render_widget_host_view_child_frame.cc (right): https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... content/browser/frame_host/render_widget_host_view_child_frame.cc:48: GetSurfaceIdNamespace(), this); On 2016/02/25 00:58:04, kenrb wrote: > Should this be replaced by a call to RegisterSurfaceIdNamespace()? That's reasonable, done. https://codereview.chromium.org/1729373003/diff/60001/content/browser/frame_h... content/browser/frame_host/render_widget_host_view_child_frame.cc:213: GetSurfaceIdNamespace()); On 2016/02/25 00:58:04, kenrb wrote: > Replace with a call to UnregisterSurfaceIdNamespace()? This is going away in https://codereview.chromium.org/1711103002/ which will land before this CL (I hope!) https://codereview.chromium.org/1729373003/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp (left): https://codereview.chromium.org/1729373003/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp:321: On 2016/02/25 00:58:04, kenrb wrote: > This probably isn't worth touching Blink, I'm guessing it wasn't deliberate? Done. 
 The CQ bit was unchecked by commit-bot@chromium.org 
 Dry run: This issue passed the CQ dry run. 
 kenrb@chromium.org changed reviewers: + piman@chromium.org 
 This looks okay to me. piman: Are you able to review this for James? 
 https://codereview.chromium.org/1729373003/diff/100001/content/browser/frame_... File content/browser/frame_host/render_widget_host_view_child_frame.cc (right): https://codereview.chromium.org/1729373003/diff/100001/content/browser/frame_... content/browser/frame_host/render_widget_host_view_child_frame.cc:46: RegisterSurfaceNamespaceId(); Where is the corresponding UnregisteSurfaceNamespaceId()? This passes |this| to the router, so there needs to be something to unregister before this instance goes away. https://codereview.chromium.org/1729373003/diff/100001/content/browser/render... File content/browser/renderer_host/render_widget_host_view_base.h (right): https://codereview.chromium.org/1729373003/diff/100001/content/browser/render... content/browser/renderer_host/render_widget_host_view_base.h:364: virtual void RegisterSurfaceNamespaceId() {} Does this need to be virtual? AFAICT, the only callers are: - browser_plugin_guest.cc and web_contents_impl.cc, which call it on the concrete type - RWHVChildFrame's constructor, which concerns me - calling a virtual method from a constructor is dangerous and can cause subtle bugs. 
 The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run 
 Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/140001 
 Comments addressed, ptal? (Do we need a new reviewer as piman@ is away?) https://codereview.chromium.org/1729373003/diff/100001/content/browser/frame_... File content/browser/frame_host/render_widget_host_view_child_frame.cc (right): https://codereview.chromium.org/1729373003/diff/100001/content/browser/frame_... content/browser/frame_host/render_widget_host_view_child_frame.cc:46: RegisterSurfaceNamespaceId(); On 2016/02/25 21:27:21, piman - OOO back 2016-03-04 wrote: > Where is the corresponding UnregisteSurfaceNamespaceId()? This passes |this| to > the router, so there needs to be something to unregister before this instance > goes away. It is called in BrowserPluginGuest. However, it should be noted that because of the complicated lifetime relationship between RWHVx and RenderWidgetHostInputEventRouter (they can be destructed in either order in the general case), the removal of RWHVx entries in the RenderWidgetHostInputEventRouter owner_map_ is now (as of https://codereview.chromium.org/1711103002) handled by implementing RenderWidgetHostViewBaseObserver and having RWHIER remove the entries as it observes the RWHVx's destruction, otherwise the entries are removed as RWHIER itself destructs. https://codereview.chromium.org/1729373003/diff/100001/content/browser/render... File content/browser/renderer_host/render_widget_host_view_base.h (right): https://codereview.chromium.org/1729373003/diff/100001/content/browser/render... content/browser/renderer_host/render_widget_host_view_base.h:364: virtual void RegisterSurfaceNamespaceId() {} On 2016/02/25 21:27:21, piman - OOO back 2016-03-04 wrote: > Does this need to be virtual? AFAICT, the only callers are: > - browser_plugin_guest.cc and web_contents_impl.cc, which call it on the > concrete type > - RWHVChildFrame's constructor, which concerns me - calling a virtual method > from a constructor is dangerous and can cause subtle bugs. No, we can make it concrete on RenderWidgetHostViewChildFrame. Done. 
 The CQ bit was unchecked by commit-bot@chromium.org 
 Dry run: This issue passed the CQ dry run. 
 nasko@chromium.org changed reviewers: + nasko@chromium.org 
 Looks good and I'm happy to rubberstamp if kenrb@ is happy with the CL. Some nits included. https://codereview.chromium.org/1729373003/diff/140001/content/browser/frame_... File content/browser/frame_host/cross_process_frame_connector.cc (right): https://codereview.chromium.org/1729373003/diff/140001/content/browser/frame_... content/browser/frame_host/cross_process_frame_connector.cc:155: if (auto main_view = GetRootRenderWidgetHostView()) nit: Variable declaration should be outside the if statement. https://codereview.chromium.org/1729373003/diff/140001/content/browser/frame_... content/browser/frame_host/cross_process_frame_connector.cc:166: void CrossProcessFrameConnector::Focus() { Should this be named FocusRoot? Or something similar? Or maybe add a comment explaining the behavior. https://codereview.chromium.org/1729373003/diff/140001/content/browser/site_p... File content/browser/site_per_process_browsertest.cc (right): https://codereview.chromium.org/1729373003/diff/140001/content/browser/site_p... content/browser/site_per_process_browsertest.cc:4445: "a.com", "/frame_tree/page_with_one_frame.html")); You can navigate to "/cross_site_iframe_factory.html?a(b)", which will give you an a.com parent with b.com child in one navigation. This will avoid having to navigate separately the subframe below. https://codereview.chromium.org/1729373003/diff/140001/content/browser/site_p... content/browser/site_per_process_browsertest.cc:4453: embedded_test_server()->GetURL("b.com", "/page_with_touch_handler.html")); Ah, you need a specific page in the iframe. Feel free to disregard my previous comment, though I'm going to leave it for information purposes. https://codereview.chromium.org/1729373003/diff/140001/content/browser/site_p... content/browser/site_per_process_browsertest.cc:4467: nit: no need for extra empty line 
 On 2016/02/29 22:10:26, nasko wrote: > Looks good and I'm happy to rubberstamp if kenrb@ is happy with the CL. Some > nits included. > > https://codereview.chromium.org/1729373003/diff/140001/content/browser/frame_... > File content/browser/frame_host/cross_process_frame_connector.cc (right): > > https://codereview.chromium.org/1729373003/diff/140001/content/browser/frame_... > content/browser/frame_host/cross_process_frame_connector.cc:155: if (auto > main_view = GetRootRenderWidgetHostView()) > nit: Variable declaration should be outside the if statement. > > https://codereview.chromium.org/1729373003/diff/140001/content/browser/frame_... > content/browser/frame_host/cross_process_frame_connector.cc:166: void > CrossProcessFrameConnector::Focus() { > Should this be named FocusRoot? Or something similar? Or maybe add a comment > explaining the behavior. > > https://codereview.chromium.org/1729373003/diff/140001/content/browser/site_p... > File content/browser/site_per_process_browsertest.cc (right): > > https://codereview.chromium.org/1729373003/diff/140001/content/browser/site_p... > content/browser/site_per_process_browsertest.cc:4445: "a.com", > "/frame_tree/page_with_one_frame.html")); > You can navigate to "/cross_site_iframe_factory.html?a(b)", which will give you > an a.com parent with b.com child in one navigation. This will avoid having to > navigate separately the subframe below. > > https://codereview.chromium.org/1729373003/diff/140001/content/browser/site_p... > content/browser/site_per_process_browsertest.cc:4453: > embedded_test_server()->GetURL("b.com", "/page_with_touch_handler.html")); > Ah, you need a specific page in the iframe. Feel free to disregard my previous > comment, though I'm going to leave it for information purposes. > > https://codereview.chromium.org/1729373003/diff/140001/content/browser/site_p... > content/browser/site_per_process_browsertest.cc:4467: > nit: no need for extra empty line kenrb@ indicated he was OK with it in #27. I'll fix the nits you mention and upload a new CL shortly. 
 Nits addressed. https://codereview.chromium.org/1729373003/diff/140001/content/browser/frame_... File content/browser/frame_host/cross_process_frame_connector.cc (right): https://codereview.chromium.org/1729373003/diff/140001/content/browser/frame_... content/browser/frame_host/cross_process_frame_connector.cc:155: if (auto main_view = GetRootRenderWidgetHostView()) On 2016/02/29 22:10:26, nasko wrote: > nit: Variable declaration should be outside the if statement. Done. https://codereview.chromium.org/1729373003/diff/140001/content/browser/frame_... content/browser/frame_host/cross_process_frame_connector.cc:166: void CrossProcessFrameConnector::Focus() { On 2016/02/29 22:10:26, nasko wrote: > Should this be named FocusRoot? Or something similar? Or maybe add a comment > explaining the behavior. Done. https://codereview.chromium.org/1729373003/diff/140001/content/browser/site_p... File content/browser/site_per_process_browsertest.cc (right): https://codereview.chromium.org/1729373003/diff/140001/content/browser/site_p... content/browser/site_per_process_browsertest.cc:4445: "a.com", "/frame_tree/page_with_one_frame.html")); On 2016/02/29 22:10:26, nasko wrote: > You can navigate to "/cross_site_iframe_factory.html?a(b)", which will give you > an a.com parent with b.com child in one navigation. This will avoid having to > navigate separately the subframe below. Acknowledged. https://codereview.chromium.org/1729373003/diff/140001/content/browser/site_p... content/browser/site_per_process_browsertest.cc:4453: embedded_test_server()->GetURL("b.com", "/page_with_touch_handler.html")); On 2016/02/29 22:10:26, nasko wrote: > Ah, you need a specific page in the iframe. Feel free to disregard my previous > comment, though I'm going to leave it for information purposes. Done. https://codereview.chromium.org/1729373003/diff/140001/content/browser/site_p... content/browser/site_per_process_browsertest.cc:4467: On 2016/02/29 22:10:26, nasko wrote: > nit: no need for extra empty line Done. 
 The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run 
 Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/160001 
 Patchset #9 (id:160001) has been deleted 
 Dry run: Try jobs failed on following builders: ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...) ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...) mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...) mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...) 
 The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run 
 Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/180001 
 LGTM, assuming kenrb@ also stamps an LGTM. I know he said it looks fine, but it will be nice to be explicit. 
 The CQ bit was unchecked by commit-bot@chromium.org 
 Dry run: This issue passed the CQ dry run. 
 kenrb@ - If you get a moment, could you take another look? 
 The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run 
 Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/200001 
 lgtm 
 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 wjmaclean@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/1729373003/#ps200001 (title: "Add RenderWidgetHostViewMac::ProcessTouchEvent().") 
 CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/200001 
 The CQ bit was unchecked by wjmaclean@chromium.org 
 The CQ bit was unchecked by commit-bot@chromium.org 
 The CQ bit was checked by wjmaclean@chromium.org 
 The patchset sent to the CQ was uploaded after l-g-t-m from kenrb@chromium.org, nasko@chromium.org Link to the patchset: https://codereview.chromium.org/1729373003/#ps220001 (title: "Fix compile guard for test.") 
 CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/220001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/220001 
 The CQ bit was unchecked by commit-bot@chromium.org 
 Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) 
 The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run 
 Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/240001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/240001 
 The CQ bit was unchecked by commit-bot@chromium.org 
 Dry run: Try jobs failed on following builders: cast_shell_linux on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) 
 The CQ bit was checked by wjmaclean@chromium.org 
 The patchset sent to the CQ was uploaded after l-g-t-m from kenrb@chromium.org, nasko@chromium.org Link to the patchset: https://codereview.chromium.org/1729373003/#ps240001 (title: "Try different synchronization for hit-testing in test.") 
 CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/240001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/240001 
 The CQ bit was unchecked by commit-bot@chromium.org 
 Try jobs failed on following builders: cast_shell_linux on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) 
 The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run 
 Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/260001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/260001 
 The CQ bit was unchecked by commit-bot@chromium.org 
 Dry run: Try jobs failed on following builders: cast_shell_linux on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) 
 The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run 
 Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/280001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/280001 
 Patchset #13 (id:260001) has been deleted 
 The CQ bit was unchecked by commit-bot@chromium.org 
 Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...) 
 The CQ bit was checked by wjmaclean@chromium.org 
 The patchset sent to the CQ was uploaded after l-g-t-m from kenrb@chromium.org, nasko@chromium.org Link to the patchset: https://codereview.chromium.org/1729373003/#ps280001 (title: "Use positioned iframe in test to simplify event targeting.") 
 CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1729373003/280001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1729373003/280001 
 
            
              
                Message was sent while issue was closed.
              
            
             Description was changed from ========== Implement touch events for site-isolation. This CL implements the necessary machinery to plumb touch events into oopif frames, including oopif-based WebView (BrowserTag). BUG=581892 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation ========== to ========== Implement touch events for site-isolation. This CL implements the necessary machinery to plumb touch events into oopif frames, including oopif-based WebView (BrowserTag). BUG=581892 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation ========== 
 
            
              
                Message was sent while issue was closed.
              
            
             Committed patchset #13 (id:280001) 
 
            
              
                Message was sent while issue was closed.
              
            
             Description was changed from ========== Implement touch events for site-isolation. This CL implements the necessary machinery to plumb touch events into oopif frames, including oopif-based WebView (BrowserTag). BUG=581892 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation ========== to ========== Implement touch events for site-isolation. This CL implements the necessary machinery to plumb touch events into oopif frames, including oopif-based WebView (BrowserTag). BUG=581892 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation Committed: https://crrev.com/fab616ad24c87bcd843a77dcbdf7fb299c635cf2 Cr-Commit-Position: refs/heads/master@{#378781} ========== 
 
            
              
                Message was sent while issue was closed.
              
            
             Patchset 13 (id:??) landed as https://crrev.com/fab616ad24c87bcd843a77dcbdf7fb299c635cf2 Cr-Commit-Position: refs/heads/master@{#378781} | 
