Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2524863004:
Fix bug where CSS styles are requested after node is destroyed (Closed)
|
Created:
4 years ago by Sarmad Hashmi Modified:
4 years ago Reviewers:
sadrul CC:
chromium-reviews, kalyank, sadrul, pfeldman, devtools-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix bug where CSS styles are requested after node is destroyed
There's a bug which causes a crash when a node is highlighted in the
inspector and the corresponding object is destroyed in Ash. This somehow
triggers the frontend to request the styles for that node again, so
instead of NOTREACHED(), we simply return an error.
BUG=648701
Committed: https://crrev.com/dd0bb09f4fd6d80996b147bec0a1c6b142d5d42f
Cr-Commit-Position: refs/heads/master@{#434563}
Patch Set 1 #
Total comments: 6
Patch Set 2 : sadruls comments #Messages
Total messages: 19 (13 generated)
The CQ bit was checked by mhashmi@chromium.org to run a CQ dry run
mhashmi@chromium.org changed reviewers: + sadrul@chromium.org
PTAL sadrul@. Extremely minor change.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
lgtm https://codereview.chromium.org/2524863004/diff/1/ash/common/devtools/ash_dev... File ash/common/devtools/ash_devtools_css_agent.cc (right): https://codereview.chromium.org/2524863004/diff/1/ash/common/devtools/ash_dev... ash/common/devtools/ash_devtools_css_agent.cc:67: int nodeId, node_id https://codereview.chromium.org/2524863004/diff/1/ash/common/devtools/ash_dev... ash/common/devtools/ash_devtools_css_agent.cc:73: "Node with that id not found"); Add {} https://codereview.chromium.org/2524863004/diff/1/ash/common/devtools/ash_dev... ash/common/devtools/ash_devtools_css_agent.cc:78: AshDevToolsCSSAgent::GetStylesForNode(int nodeId) { Whoops: node_id
The CQ bit was checked by mhashmi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2524863004/diff/1/ash/common/devtools/ash_dev... File ash/common/devtools/ash_devtools_css_agent.cc (right): https://codereview.chromium.org/2524863004/diff/1/ash/common/devtools/ash_dev... ash/common/devtools/ash_devtools_css_agent.cc:67: int nodeId, On 2016/11/25 18:30:33, sadrul wrote: > node_id Done. https://codereview.chromium.org/2524863004/diff/1/ash/common/devtools/ash_dev... ash/common/devtools/ash_devtools_css_agent.cc:73: "Node with that id not found"); On 2016/11/25 18:30:32, sadrul wrote: > Add {} Done. https://codereview.chromium.org/2524863004/diff/1/ash/common/devtools/ash_dev... ash/common/devtools/ash_devtools_css_agent.cc:78: AshDevToolsCSSAgent::GetStylesForNode(int nodeId) { On 2016/11/25 18:30:32, sadrul wrote: > Whoops: node_id Done.
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 mhashmi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org Link to the patchset: https://codereview.chromium.org/2524863004/#ps20001 (title: "sadruls comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480128364619810,
"parent_rev": "2376ebce29cbba59256876a6cdddca6bd0a4c5e2", "commit_rev":
"d5cdee032d751316a789eb7c2c4c4f7ec036bca5"}
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Fix bug where CSS styles are requested after node is destroyed There's a bug which causes a crash when a node is highlighted in the inspector and the corresponding object is destroyed in Ash. This somehow triggers the frontend to request the styles for that node again, so instead of NOTREACHED(), we simply return an error. BUG=648701 ========== to ========== Fix bug where CSS styles are requested after node is destroyed There's a bug which causes a crash when a node is highlighted in the inspector and the corresponding object is destroyed in Ash. This somehow triggers the frontend to request the styles for that node again, so instead of NOTREACHED(), we simply return an error. BUG=648701 Committed: https://crrev.com/dd0bb09f4fd6d80996b147bec0a1c6b142d5d42f Cr-Commit-Position: refs/heads/master@{#434563} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/dd0bb09f4fd6d80996b147bec0a1c6b142d5d42f Cr-Commit-Position: refs/heads/master@{#434563} |
