Browse Source

chore: cherry-pick c768895b3f, 58393127e7 and c2f6803bdd from chromium (#28821)

* chore: cherry-pick c768895b3f, 58393127e7 and c2f6803bdd from chromium

* update patches

Co-authored-by: Electron Bot <[email protected]>
Pedro Pontes 4 years ago
parent
commit
cd093d2359

+ 3 - 0
patches/chromium/.patches

@@ -117,3 +117,6 @@ cherry-pick-66b9ad64f4a4.patch
 cherry-pick-7dd3b1c86795.patch
 cherry-pick-1536a564d959.patch
 cherry-pick-e4abe032f3ad.patch
+add_crashkeys_to_identify_where_target_is_assigned_to_a_stale_value.patch
+add_weak_pointer_to_rwhier_framesinkidownermap_and_rwhier_targetmap.patch
+add_null_pointer_check_in_renderwidgethostinputeventrouter.patch

+ 85 - 0
patches/chromium/add_crashkeys_to_identify_where_target_is_assigned_to_a_stale_value.patch

@@ -0,0 +1,85 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Lan Wei <[email protected]>
+Date: Thu, 15 Apr 2021 20:01:17 +0000
+Subject: Add crashkeys to identify where |target| is assigned to a stale value
+
+In RenderWidgetHostInputEventRouter::DispatchTouchscreenGestureEvent, the
+|target|'s address is changed and assigned to a stale value.
+
+(cherry picked from commit b7758233216445264174dd249e7565ab4849daa6)
+
+Bug: 1155297
+Change-Id: Id87175059b6d74eeac165abe0ccfd5f6c25d659a
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2764892
+Commit-Queue: Lan Wei <[email protected]>
+Reviewed-by: Alex Moshchuk <[email protected]>
+Reviewed-by: James MacLean <[email protected]>
+Cr-Original-Commit-Position: refs/heads/master@{#867419}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2828850
+Auto-Submit: Lan Wei <[email protected]>
+Reviewed-by: Adrian Taylor <[email protected]>
+Owners-Override: Lan Wei <[email protected]>
+Cr-Commit-Position: refs/branch-heads/4430@{#1292}
+Cr-Branched-From: e5ce7dc4f7518237b3d9bb93cccca35d25216cbe-refs/heads/master@{#857950}
+
+diff --git a/content/browser/renderer_host/render_widget_host_input_event_router.cc b/content/browser/renderer_host/render_widget_host_input_event_router.cc
+index 10399e2f1dc63cf3715ad887f20b8b46ce0d4e5d..c028af696bfcb9670745a960a0f5ddb19f2c8174 100644
+--- a/content/browser/renderer_host/render_widget_host_input_event_router.cc
++++ b/content/browser/renderer_host/render_widget_host_input_event_router.cc
+@@ -1512,6 +1512,10 @@ void RenderWidgetHostInputEventRouter::DispatchTouchscreenGestureEvent(
+ 
+   base::Optional<gfx::PointF> fallback_target_location;
+ 
++  // Adding crash logs to track the reason of stale pointer value of |target|.
++  LogTouchscreenGestureTargetCrashKeys(
++      "RWHIER::DispatchTouchscreenGestureEvent target set from caller");
++
+   if (gesture_event.unique_touch_event_id == 0) {
+     // On Android it is possible for touchscreen gesture events to arrive that
+     // are not associated with touch events, because non-synthetic events can be
+@@ -1538,9 +1542,19 @@ void RenderWidgetHostInputEventRouter::DispatchTouchscreenGestureEvent(
+     // don't worry about the fact we're ignoring |result.should_query_view|, as
+     // this is the best we can do until we fix https://crbug.com/595422.
+     target = result.view;
++
++    // Adding crash logs to track the reason of stale pointer value of |target|.
++    LogTouchscreenGestureTargetCrashKeys(
++        "RWHIER::DispatchTouchscreenGestureEvent target from "
++        "FindViewAtLocation");
+     fallback_target_location = transformed_point;
+   } else if (is_gesture_start) {
+     target = gesture_target_it->second;
++
++    // Adding crash logs to track the reason of stale pointer value of |target|.
++    LogTouchscreenGestureTargetCrashKeys(
++        "RWHIER::DispatchTouchscreenGestureEvent target from "
++        "touchscreen_gesture_target_map_");
+     touchscreen_gesture_target_map_.erase(gesture_target_it);
+ 
+     // Abort any scroll bubbling in progress to avoid double entry.
+@@ -1969,4 +1983,11 @@ void RenderWidgetHostInputEventRouter::SetAutoScrollInProgress(
+   event_targeter_->SetIsAutoScrollInProgress(is_autoscroll_in_progress);
+ }
+ 
++void RenderWidgetHostInputEventRouter::LogTouchscreenGestureTargetCrashKeys(
++    const std::string& log_message) {
++  static auto* target_crash_key = base::debug::AllocateCrashKeyString(
++      "target_crash_key", base::debug::CrashKeySize::Size256);
++  base::debug::SetCrashKeyString(target_crash_key, log_message);
++}
++
+ }  // namespace content
+diff --git a/content/browser/renderer_host/render_widget_host_input_event_router.h b/content/browser/renderer_host/render_widget_host_input_event_router.h
+index 87a63125228b7346841f63f580157cb6167d8fde..dc9b0d5ea1118f53897307b5f17e253dbe283579 100644
+--- a/content/browser/renderer_host/render_widget_host_input_event_router.h
++++ b/content/browser/renderer_host/render_widget_host_input_event_router.h
+@@ -332,6 +332,9 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter final
+   void SetTouchscreenGestureTarget(RenderWidgetHostViewBase* target,
+                                    bool moved_recently = false);
+ 
++  // TODO(crbug.com/1155297): Remove when bug investigation is complete.
++  void LogTouchscreenGestureTargetCrashKeys(const std::string& log_message);
++
+   FrameSinkIdOwnerMap owner_map_;
+   TargetMap touchscreen_gesture_target_map_;
+   RenderWidgetHostViewBase* touch_target_ = nullptr;

+ 36 - 0
patches/chromium/add_null_pointer_check_in_renderwidgethostinputeventrouter.patch

@@ -0,0 +1,36 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Lan Wei <[email protected]>
+Date: Fri, 16 Apr 2021 03:49:10 +0000
+Subject: Add null pointer check in RenderWidgetHostInputEventRouter
+
+We have some crashes in RenderWidgetHostInputEventRouter class, we are
+adding some null pointer check in this class to avoid the crash.
+
+(cherry picked from commit 5f47666b79ac7ded20e1c7657037498561bd3352)
+
+Bug: 1155297
+Change-Id: I3b63d5748523ae2ce8ab469832adfc75d586e411
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2818680
+Reviewed-by: Charlie Reis <[email protected]>
+Commit-Queue: Lan Wei <[email protected]>
+Cr-Original-Commit-Position: refs/heads/master@{#871108}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2830091
+Auto-Submit: Lan Wei <[email protected]>
+Bot-Commit: Rubber Stamper <[email protected]>
+Owners-Override: Lan Wei <[email protected]>
+Cr-Commit-Position: refs/branch-heads/4430@{#1296}
+Cr-Branched-From: e5ce7dc4f7518237b3d9bb93cccca35d25216cbe-refs/heads/master@{#857950}
+
+diff --git a/content/browser/renderer_host/render_widget_host_input_event_router.cc b/content/browser/renderer_host/render_widget_host_input_event_router.cc
+index dfd11a52b2d7e8724423ffb37686e5904a96e861..7e309490d675c5dbebe447896b30f0d9cc0bd1d6 100644
+--- a/content/browser/renderer_host/render_widget_host_input_event_router.cc
++++ b/content/browser/renderer_host/render_widget_host_input_event_router.cc
+@@ -1958,7 +1958,7 @@ void RenderWidgetHostInputEventRouter::OnAggregatedHitTestRegionListUpdated(
+     const std::vector<viz::AggregatedHitTestRegion>& hit_test_data) {
+   for (auto& region : hit_test_data) {
+     auto iter = owner_map_.find(region.frame_sink_id);
+-    if (iter != owner_map_.end())
++    if (iter != owner_map_.end() && iter->second)
+       iter->second->NotifyHitTestRegionUpdated(region);
+   }
+ }

+ 167 - 0
patches/chromium/add_weak_pointer_to_rwhier_framesinkidownermap_and_rwhier_targetmap.patch

@@ -0,0 +1,167 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Lan Wei <[email protected]>
+Date: Thu, 15 Apr 2021 23:11:30 +0000
+Subject: Add weak pointer to RWHIER::FrameSinkIdOwnerMap and RWHIER::TargetMap
+
+In RWHIER::FrameSinkIdOwnerMap and RWHIER::TargetMap, we change raw
+pointer of RenderWidgetHostViewBase to weak pointer, such as
+using FrameSinkIdOwnerMap = std::unordered_map<viz::FrameSinkId,
+                                base::WeakPtr<RenderWidgetHostViewBase>,
+                                viz::FrameSinkIdHash>;
+using TargetMap = std::map<uint32_t,
+                           base::WeakPtr<RenderWidgetHostViewBase>>;
+
+This CL should fix the crash of stale pointer.
+
+(cherry picked from commit 3e3e3cf7036d7e33a4d68b8416ae25730f9eee1d)
+
+Bug: 1155297
+Change-Id: I5b3270882ef06ae48c86bd460261723c7113953d
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2792344
+Reviewed-by: James MacLean <[email protected]>
+Reviewed-by: Aaron Colwell <[email protected]>
+Commit-Queue: Lan Wei <[email protected]>
+Cr-Original-Commit-Position: refs/heads/master@{#870013}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2828858
+Auto-Submit: Lan Wei <[email protected]>
+Bot-Commit: Rubber Stamper <[email protected]>
+Owners-Override: Lan Wei <[email protected]>
+Cr-Commit-Position: refs/branch-heads/4430@{#1293}
+Cr-Branched-From: e5ce7dc4f7518237b3d9bb93cccca35d25216cbe-refs/heads/master@{#857950}
+
+diff --git a/content/browser/renderer_host/render_widget_host_input_event_router.cc b/content/browser/renderer_host/render_widget_host_input_event_router.cc
+index c028af696bfcb9670745a960a0f5ddb19f2c8174..dfd11a52b2d7e8724423ffb37686e5904a96e861 100644
+--- a/content/browser/renderer_host/render_widget_host_input_event_router.cc
++++ b/content/browser/renderer_host/render_widget_host_input_event_router.cc
+@@ -335,7 +335,7 @@ void RenderWidgetHostInputEventRouter::OnRenderWidgetHostViewBaseDestroyed(
+ 
+   // Remove this view from the owner_map.
+   for (auto entry : owner_map_) {
+-    if (entry.second == view) {
++    if (entry.second.get() == view) {
+       owner_map_.erase(entry.first);
+       // There will only be one instance of a particular view in the map.
+       break;
+@@ -358,7 +358,7 @@ void RenderWidgetHostInputEventRouter::OnRenderWidgetHostViewBaseDestroyed(
+   // replace it with nullptr so that we maintain the 1:1 correspondence between
+   // map entries and the touch sequences that underly them.
+   for (auto it : touchscreen_gesture_target_map_) {
+-    if (it.second == view)
++    if (it.second.get() == view)
+       it.second = nullptr;
+   }
+ 
+@@ -407,8 +407,10 @@ void RenderWidgetHostInputEventRouter::OnRenderWidgetHostViewBaseDestroyed(
+ void RenderWidgetHostInputEventRouter::ClearAllObserverRegistrations() {
+   // Since we're shutting down, it's safe to call RenderWidgetHostViewBase::
+   // RemoveObserver() directly here.
+-  for (auto entry : owner_map_)
+-    entry.second->RemoveObserver(this);
++  for (auto entry : owner_map_) {
++    if (entry.second)
++      entry.second->RemoveObserver(this);
++  }
+   owner_map_.clear();
+   viz::HostFrameSinkManager* manager = GetHostFrameSinkManager();
+   if (manager)
+@@ -830,7 +832,7 @@ void RenderWidgetHostInputEventRouter::DispatchTouchEvent(
+                touch_event.unique_touch_event_id) ==
+            touchscreen_gesture_target_map_.end());
+     touchscreen_gesture_target_map_[touch_event.unique_touch_event_id] =
+-        touch_target_;
++        touch_target_->GetWeakPtr();
+   } else if (touch_event.GetType() == blink::WebInputEvent::Type::kTouchStart) {
+     active_touches_ += CountChangedTouchPoints(touch_event);
+   }
+@@ -1342,7 +1344,7 @@ void RenderWidgetHostInputEventRouter::AddFrameSinkIdOwner(
+   // We want to be notified if the owner is destroyed so we can remove it from
+   // our map.
+   owner->AddObserver(this);
+-  owner_map_.insert(std::make_pair(id, owner));
++  owner_map_.insert(std::make_pair(id, owner->GetWeakPtr()));
+ }
+ 
+ void RenderWidgetHostInputEventRouter::RemoveFrameSinkIdOwner(
+@@ -1354,7 +1356,8 @@ void RenderWidgetHostInputEventRouter::RemoveFrameSinkIdOwner(
+     // stale values if the view destructs and isn't an observer anymore.
+     // Note: the view the iterator points at will be deleted in the following
+     // call, and shouldn't be used after this point.
+-    OnRenderWidgetHostViewBaseDestroyed(it_to_remove->second);
++    if (it_to_remove->second)
++      OnRenderWidgetHostViewBaseDestroyed(it_to_remove->second.get());
+   }
+ }
+ 
+@@ -1405,7 +1408,7 @@ RenderWidgetHostInputEventRouter::FindTouchscreenGestureEventTarget(
+ bool RenderWidgetHostInputEventRouter::IsViewInMap(
+     const RenderWidgetHostViewBase* view) const {
+   DCHECK(!is_registered(view->GetFrameSinkId()) ||
+-         owner_map_.find(view->GetFrameSinkId())->second == view);
++         owner_map_.find(view->GetFrameSinkId())->second.get() == view);
+   return is_registered(view->GetFrameSinkId());
+ }
+ 
+@@ -1549,7 +1552,7 @@ void RenderWidgetHostInputEventRouter::DispatchTouchscreenGestureEvent(
+         "FindViewAtLocation");
+     fallback_target_location = transformed_point;
+   } else if (is_gesture_start) {
+-    target = gesture_target_it->second;
++    target = gesture_target_it->second.get();
+ 
+     // Adding crash logs to track the reason of stale pointer value of |target|.
+     LogTouchscreenGestureTargetCrashKeys(
+@@ -1740,7 +1743,7 @@ RenderWidgetHostInputEventRouter::FindViewFromFrameSinkId(
+   // If the point hit a Surface whose namspace is no longer in the map, then
+   // it likely means the RenderWidgetHostView has been destroyed but its
+   // parent frame has not sent a new compositor frame since that happened.
+-  return iter == owner_map_.end() ? nullptr : iter->second;
++  return iter == owner_map_.end() ? nullptr : iter->second.get();
+ }
+ 
+ bool RenderWidgetHostInputEventRouter::ShouldContinueHitTesting(
+@@ -1760,8 +1763,10 @@ bool RenderWidgetHostInputEventRouter::ShouldContinueHitTesting(
+ std::vector<RenderWidgetHostView*>
+ RenderWidgetHostInputEventRouter::GetRenderWidgetHostViewsForTests() const {
+   std::vector<RenderWidgetHostView*> hosts;
+-  for (auto entry : owner_map_)
+-    hosts.push_back(entry.second);
++  for (auto entry : owner_map_) {
++    DCHECK(entry.second);
++    hosts.push_back(entry.second.get());
++  }
+ 
+   return hosts;
+ }
+@@ -1930,8 +1935,10 @@ void RenderWidgetHostInputEventRouter::SetCursor(const WebCursor& cursor) {
+   last_device_scale_factor_ =
+       last_mouse_move_root_view_->current_device_scale_factor();
+   if (auto* cursor_manager = last_mouse_move_root_view_->GetCursorManager()) {
+-    for (auto it : owner_map_)
+-      cursor_manager->UpdateCursor(it.second, cursor);
++    for (auto it : owner_map_) {
++      if (it.second)
++        cursor_manager->UpdateCursor(it.second.get(), cursor);
++    }
+   }
+ }
+ 
+diff --git a/content/browser/renderer_host/render_widget_host_input_event_router.h b/content/browser/renderer_host/render_widget_host_input_event_router.h
+index dc9b0d5ea1118f53897307b5f17e253dbe283579..8d438027703e55d2ad18ea8234e0b318854c65ef 100644
+--- a/content/browser/renderer_host/render_widget_host_input_event_router.h
++++ b/content/browser/renderer_host/render_widget_host_input_event_router.h
+@@ -195,10 +195,11 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter final
+   FRIEND_TEST_ALL_PREFIXES(BrowserSideFlingBrowserTest,
+                            InertialGSUBubblingStopsWhenParentCannotScroll);
+ 
+-  using FrameSinkIdOwnerMap = std::unordered_map<viz::FrameSinkId,
+-                                                 RenderWidgetHostViewBase*,
+-                                                 viz::FrameSinkIdHash>;
+-  using TargetMap = std::map<uint32_t, RenderWidgetHostViewBase*>;
++  using FrameSinkIdOwnerMap =
++      std::unordered_map<viz::FrameSinkId,
++                         base::WeakPtr<RenderWidgetHostViewBase>,
++                         viz::FrameSinkIdHash>;
++  using TargetMap = std::map<uint32_t, base::WeakPtr<RenderWidgetHostViewBase>>;
+ 
+   void ClearAllObserverRegistrations();
+   RenderWidgetTargetResult FindViewAtLocation(