-
Notifications
You must be signed in to change notification settings - Fork 15k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: cherry-pick c768895b3f, 58393127e7 and c2f6803bdd from chromium
- Loading branch information
Showing
4 changed files
with
292 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
85 changes: 85 additions & 0 deletions
85
patches/chromium/add_crashkeys_to_identify_where_target_is_assigned_to_a_stale_value.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
| From: Lan Wei <lanwei@chromium.org> | ||
| 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 <lanwei@chromium.org> | ||
| Reviewed-by: Alex Moshchuk <alexmos@chromium.org> | ||
| Reviewed-by: James MacLean <wjmaclean@chromium.org> | ||
| Cr-Original-Commit-Position: refs/heads/master@{#867419} | ||
| Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2828850 | ||
| Auto-Submit: Lan Wei <lanwei@chromium.org> | ||
| Reviewed-by: Adrian Taylor <adetaylor@google.com> | ||
| Owners-Override: Lan Wei <lanwei@chromium.org> | ||
| 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 changes: 36 additions & 0 deletions
36
patches/chromium/add_null_pointer_check_in_renderwidgethostinputeventrouter.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
| From: Lan Wei <lanwei@chromium.org> | ||
| 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 <creis@chromium.org> | ||
| Commit-Queue: Lan Wei <lanwei@chromium.org> | ||
| Cr-Original-Commit-Position: refs/heads/master@{#871108} | ||
| Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2830091 | ||
| Auto-Submit: Lan Wei <lanwei@chromium.org> | ||
| Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> | ||
| Owners-Override: Lan Wei <lanwei@chromium.org> | ||
| 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); | ||
| } | ||
| } |
168 changes: 168 additions & 0 deletions
168
patches/chromium/add_weak_pointer_to_rwhier_framesinkidownermap_and_rwhier_targetmap.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
| From: Lan Wei <lanwei@chromium.org> | ||
| 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 <wjmaclean@chromium.org> | ||
| Reviewed-by: Aaron Colwell <acolwell@chromium.org> | ||
| Commit-Queue: Lan Wei <lanwei@chromium.org> | ||
| Cr-Original-Commit-Position: refs/heads/master@{#870013} | ||
| Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2828858 | ||
| Auto-Submit: Lan Wei <lanwei@chromium.org> | ||
| Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> | ||
| Owners-Override: Lan Wei <lanwei@chromium.org> | ||
| 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( |