Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2720823004:
React to non commited render frame navigations in devtools. (Closed)
|
Created:
3 years, 9 months ago by mharanczyk Modified:
3 years, 9 months ago CC:
chromium-reviews, darin-cc_chromium.org, pfeldman, devtools-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionReact to not committed render frame navigations in devtools.
Currently it was trying to handle only error pages or committed
navigations which lead to RenderFrameDevToolsAgentHost being stuck
in suspended state for navigations that did not commit (for example
downloads). This in turn caused webdriver to get stuck on further
interactions with such frames. This is regression caused by
https://codereview.chromium.org/2655413002.
BUG=682002
Review-Url: https://codereview.chromium.org/2720823004
Cr-Commit-Position: refs/heads/master@{#454229}
Committed: https://chromium.googlesource.com/chromium/src/+/b9905150f10743251466b608024ca2081f070498
Patch Set 1 #Patch Set 2 : Added test. #
Messages
Total messages: 22 (12 generated)
Description was changed from ========== React to non commited render frame navigations in devtools. Currently it was trying to handle only error pages or commited navigations which lead to RenderFrameDevToolsAgentHost being stuck in suspended state for navigations that did not commit (for example downloads). This in turn caused webdriver to get stuck on further interactions with such frames. This is regression caused by https://codereview.chromium.org/2655413002. BUG=682002 ========== to ========== React to non commited render frame navigations in devtools. Currently it was trying to handle only error pages or commited navigations which lead to RenderFrameDevToolsAgentHost being stuck in suspended state for navigations that did not commit (for example downloads). This in turn caused webdriver to get stuck on further interactions with such frames. This is regression caused by https://codereview.chromium.org/2655413002. BUG=682002 ==========
mharanczyk@opera.com changed reviewers: + dgozman@chromium.org, jam@chromium.org
jam@, dgozman@ please take a look. Before https://codereview.chromium.org/2655413002 and removal of deprecated methods DidFailProvisionalLoad was called for non committed navigations. When moving code to new function it was put into IsErrorPage condition which doesn't handle all cases it previously was triggered for.
The CQ bit was checked by jam@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...
lgtm, thanks. Is it possible to write a test? That would give us confidence that this works with PlzNavigate.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
+1 for test, if possible. @jam: I wonder whether other places had suffered from the same problem during migration.
On 2017/02/28 18:55:01, dgozman wrote: > +1 for test, if possible. > > @jam: I wonder whether other places had suffered from the same problem during > migration. Good question; I had gone through the conversions that I did in a few weeks and I didn't see other instances of this bug.
Added browser test, please take another look. Few remarks: I had to use cross process download navigation to make agent go into hung state, navigations that ended up in same host went into slightly different code paths that did not trigger the bug. This also required to use something else than about:blank as start test page as it was not triggering render frame host switch. I had used download as not committing navigation, if you know anything easier to use I could change it. Since the test was so similar to other site-per-process tests I decided to put it into same file to avoid code duplication. Current test solution was the only one that I found failing (well timeouting to be precise) without bug fix and passing with it.
Description was changed from ========== React to non commited render frame navigations in devtools. Currently it was trying to handle only error pages or commited navigations which lead to RenderFrameDevToolsAgentHost being stuck in suspended state for navigations that did not commit (for example downloads). This in turn caused webdriver to get stuck on further interactions with such frames. This is regression caused by https://codereview.chromium.org/2655413002. BUG=682002 ========== to ========== React to not committed render frame navigations in devtools. Currently it was trying to handle only error pages or committed navigations which lead to RenderFrameDevToolsAgentHost being stuck in suspended state for navigations that did not commit (for example downloads). This in turn caused webdriver to get stuck on further interactions with such frames. This is regression caused by https://codereview.chromium.org/2655413002. BUG=682002 ==========
lgtm, thank you!
The CQ bit was checked by mharanczyk@opera.com
The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org Link to the patchset: https://codereview.chromium.org/2720823004/#ps20001 (title: "Added test.")
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": 1488448339952100,
"parent_rev": "3db897f00b693287702dc2b88c5ebe757e2babaf", "commit_rev":
"b9905150f10743251466b608024ca2081f070498"}
Message was sent while issue was closed.
Description was changed from ========== React to not committed render frame navigations in devtools. Currently it was trying to handle only error pages or committed navigations which lead to RenderFrameDevToolsAgentHost being stuck in suspended state for navigations that did not commit (for example downloads). This in turn caused webdriver to get stuck on further interactions with such frames. This is regression caused by https://codereview.chromium.org/2655413002. BUG=682002 ========== to ========== React to not committed render frame navigations in devtools. Currently it was trying to handle only error pages or committed navigations which lead to RenderFrameDevToolsAgentHost being stuck in suspended state for navigations that did not commit (for example downloads). This in turn caused webdriver to get stuck on further interactions with such frames. This is regression caused by https://codereview.chromium.org/2655413002. BUG=682002 Review-Url: https://codereview.chromium.org/2720823004 Cr-Commit-Position: refs/heads/master@{#454229} Committed: https://chromium.googlesource.com/chromium/src/+/b9905150f10743251466b608024c... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/b9905150f10743251466b608024c...
Message was sent while issue was closed.
jam@google.com changed reviewers: + jam@google.com
Message was sent while issue was closed.
thanks for the test!
Message was sent while issue was closed.
mharanczy: please see the bug https://bugs.chromium.org/p/chromium/issues/detail?id=697991 (it shows you haven't logged on to bug tracker in a while) |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
