Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1783533002:
Add code to collect crash data for https://crbug.com/592319. (Closed)
DescriptionAdd code to collect crash data for https://crbug.com/592319.
The crash reports related to this bug, which is happening very
infrequently, do not yield enough information to make an informed
guess about the cause. No repro has been found.
This CL adds code to record additional information which should show up
in minidump reports related to this crash, and that should shed light
on the underlying problem.
The amount of (temporary) additional memory required should be small,
as RenderWidgetHostInputEventRouters are not created in large numbers.
BUG=592319
Committed: https://crrev.com/86c9af236a3551c8d9a94dcfc6f4ff111626cf48
Cr-Commit-Position: refs/heads/master@{#380257}
Patch Set 1 #
Total comments: 6
Patch Set 2 : Address comments. #
Messages
Total messages: 18 (9 generated)
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/1783533002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1783533002/1
Description was changed from ========== Add code to collect crash data for https://crbug.com/592319. The crash reports related to this bug, which is happening very infrequently, do not yield enough information to make an informed guess about the cause. No repro has been fount. This CL adds code to record additional information which should show up in minidump reports related to this crash, and that should shed light on the underlying problem. The amount of (temporary) additional memory required should be small, as RenderWidgetHostInputEventRouters are not created in large numbers. BUG=592319 ========== to ========== Add code to collect crash data for https://crbug.com/592319. The crash reports related to this bug, which is happening very infrequently, do not yield enough information to make an informed guess about the cause. No repro has been fount. This CL adds code to record additional information which should show up in minidump reports related to this crash, and that should shed light on the underlying problem. The amount of (temporary) additional memory required should be small, as RenderWidgetHostInputEventRouters are not created in large numbers. BUG=592319 ==========
wjmaclean@chromium.org changed reviewers: + tdresser@chromium.org
Here is an initial draft for collecting some data on the bug. Does this look ok? Are we missing anything important?
LGTM with nit. Fix description "No repro has been fount." -> found https://codereview.chromium.org/1783533002/diff/1/content/browser/renderer_ho... File content/browser/renderer_host/render_widget_host_input_event_router.cc (right): https://codereview.chromium.org/1783533002/diff/1/content/browser/renderer_ho... content/browser/renderer_host/render_widget_host_input_event_router.cc:203: (last_gesture_event_index_ + 1) % kNumLastEventTypes; Based on the code here, isn't last_gesture_event_index actually next_gesture_event_index? https://codereview.chromium.org/1783533002/diff/1/content/browser/renderer_ho... content/browser/renderer_host/render_widget_host_input_event_router.cc:220: (last_touch_event_index_ + 1) % kNumLastEventTypes; Looks like this is next_touch_event_index_. https://codereview.chromium.org/1783533002/diff/1/content/browser/renderer_ho... File content/browser/renderer_host/render_widget_host_input_event_router.h (right): https://codereview.chromium.org/1783533002/diff/1/content/browser/renderer_ho... content/browser/renderer_host/render_widget_host_input_event_router.h:131: unsigned last_touch_event_index_; Would the dump still be easily legible if you dumped this in a separate struct? It might be a bit cleaner to keep these grouped nicely.
Description was changed from ========== Add code to collect crash data for https://crbug.com/592319. The crash reports related to this bug, which is happening very infrequently, do not yield enough information to make an informed guess about the cause. No repro has been fount. This CL adds code to record additional information which should show up in minidump reports related to this crash, and that should shed light on the underlying problem. The amount of (temporary) additional memory required should be small, as RenderWidgetHostInputEventRouters are not created in large numbers. BUG=592319 ========== to ========== Add code to collect crash data for https://crbug.com/592319. The crash reports related to this bug, which is happening very infrequently, do not yield enough information to make an informed guess about the cause. No repro has been found. This CL adds code to record additional information which should show up in minidump reports related to this crash, and that should shed light on the underlying problem. The amount of (temporary) additional memory required should be small, as RenderWidgetHostInputEventRouters are not created in large numbers. BUG=592319 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Comments addressed. https://codereview.chromium.org/1783533002/diff/1/content/browser/renderer_ho... File content/browser/renderer_host/render_widget_host_input_event_router.cc (right): https://codereview.chromium.org/1783533002/diff/1/content/browser/renderer_ho... content/browser/renderer_host/render_widget_host_input_event_router.cc:203: (last_gesture_event_index_ + 1) % kNumLastEventTypes; On 2016/03/09 19:53:45, tdresser wrote: > Based on the code here, isn't last_gesture_event_index actually > next_gesture_event_index? Done. https://codereview.chromium.org/1783533002/diff/1/content/browser/renderer_ho... content/browser/renderer_host/render_widget_host_input_event_router.cc:220: (last_touch_event_index_ + 1) % kNumLastEventTypes; On 2016/03/09 19:53:45, tdresser wrote: > Looks like this is next_touch_event_index_. Done. https://codereview.chromium.org/1783533002/diff/1/content/browser/renderer_ho... File content/browser/renderer_host/render_widget_host_input_event_router.h (right): https://codereview.chromium.org/1783533002/diff/1/content/browser/renderer_ho... content/browser/renderer_host/render_widget_host_input_event_router.h:131: unsigned last_touch_event_index_; On 2016/03/09 19:53:45, tdresser wrote: > Would the dump still be easily legible if you dumped this in a separate struct? > It might be a bit cleaner to keep these grouped nicely. Done.
The CQ bit was checked by wjmaclean@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tdresser@chromium.org Link to the patchset: https://codereview.chromium.org/1783533002/#ps20001 (title: "Address comments.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1783533002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1783533002/20001
Message was sent while issue was closed.
Description was changed from ========== Add code to collect crash data for https://crbug.com/592319. The crash reports related to this bug, which is happening very infrequently, do not yield enough information to make an informed guess about the cause. No repro has been found. This CL adds code to record additional information which should show up in minidump reports related to this crash, and that should shed light on the underlying problem. The amount of (temporary) additional memory required should be small, as RenderWidgetHostInputEventRouters are not created in large numbers. BUG=592319 ========== to ========== Add code to collect crash data for https://crbug.com/592319. The crash reports related to this bug, which is happening very infrequently, do not yield enough information to make an informed guess about the cause. No repro has been found. This CL adds code to record additional information which should show up in minidump reports related to this crash, and that should shed light on the underlying problem. The amount of (temporary) additional memory required should be small, as RenderWidgetHostInputEventRouters are not created in large numbers. BUG=592319 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Add code to collect crash data for https://crbug.com/592319. The crash reports related to this bug, which is happening very infrequently, do not yield enough information to make an informed guess about the cause. No repro has been found. This CL adds code to record additional information which should show up in minidump reports related to this crash, and that should shed light on the underlying problem. The amount of (temporary) additional memory required should be small, as RenderWidgetHostInputEventRouters are not created in large numbers. BUG=592319 ========== to ========== Add code to collect crash data for https://crbug.com/592319. The crash reports related to this bug, which is happening very infrequently, do not yield enough information to make an informed guess about the cause. No repro has been found. This CL adds code to record additional information which should show up in minidump reports related to this crash, and that should shed light on the underlying problem. The amount of (temporary) additional memory required should be small, as RenderWidgetHostInputEventRouters are not created in large numbers. BUG=592319 Committed: https://crrev.com/86c9af236a3551c8d9a94dcfc6f4ff111626cf48 Cr-Commit-Position: refs/heads/master@{#380257} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/86c9af236a3551c8d9a94dcfc6f4ff111626cf48 Cr-Commit-Position: refs/heads/master@{#380257}
Message was sent while issue was closed.
Thanks! |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
