Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2964183002:
Fixes stuck navigation when opening PWA in-scope page in CCT. (Closed)
|
Created:
3 years, 5 months ago by piotrs Modified:
3 years, 5 months ago CC:
chromium-reviews, dominickn+watch_chromium.org, lizeb+watch-custom-tabs_chromium.org, pkotwicz+watch_chromium.org, zpeng+watch_chromium.org, agrieve+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionFixes stuck navigation when opening PWA in-scope page in CCT.
The issues was happening because CustomTabActivity was requesting to load the
URL on the WebContents it received. URL loading was already happening at this
point, as navigation was initiated in the renderer earlier. It worked for URLs
with different origin, as re-requesting the URL load was causing creating new
RenderFrame in a new Renderer process. For the URL with the same origin however
the new request somehow clashed with the previous request in a way I cannot
fully explain and navigation never finished. The fix is to ask the WebContents
to resume loading instead of re-requesting it. This is also what happens in
a ChromeTabbedActivity on clicking _blank links and is much faster than what I
did in previous patch.
BUG=736995
Review-Url: https://codereview.chromium.org/2964183002
Cr-Commit-Position: refs/heads/master@{#484407}
Committed: https://chromium.googlesource.com/chromium/src/+/7bf1a0ba5782af6e950604667ecf1882b189c858
Patch Set 1 #
Total comments: 2
Patch Set 2 : Field rename for clarity in WebappNavigationTest #
Total comments: 2
Patch Set 3 : Merge #Patch Set 4 : Using "isLoading" instead of a new variable #Patch Set 5 : Merge #
Messages
Total messages: 26 (18 generated)
Description was changed from ========== Fixes stuck navigation when opening PWA in-scope page in CCT. The issues was happening because CustomTabActivity was requesting to load the URL on the WebContents it received. URL loading was already happening at this point, as navigation was initiated in the renderer earlier. It worked for URLs with different origin, as re-requesting the URL load was causing creating new RenderFrame in a new Renderer process. For the URL with the same origin however the new request somehow clashed with the previous request in a way I cannot fully explain and navigation never finished. The fix is to ask the WebContents to resume loading instead of re-requesting it. BUG=736995 ========== to ========== Fixes stuck navigation when opening PWA in-scope page in CCT. The issues was happening because CustomTabActivity was requesting to load the URL on the WebContents it received. URL loading was already happening at this point, as navigation was initiated in the renderer earlier. It worked for URLs with different origin, as re-requesting the URL load was causing creating new RenderFrame in a new Renderer process. For the URL with the same origin however the new request somehow clashed with the previous request in a way I cannot fully explain and navigation never finished. The fix is to ask the WebContents to resume loading instead of re-requesting it. This is also what happens in a ChromeTabbedActivity on clicking _blank links and is much faster than previous code. BUG=736995 ==========
Description was changed from ========== Fixes stuck navigation when opening PWA in-scope page in CCT. The issues was happening because CustomTabActivity was requesting to load the URL on the WebContents it received. URL loading was already happening at this point, as navigation was initiated in the renderer earlier. It worked for URLs with different origin, as re-requesting the URL load was causing creating new RenderFrame in a new Renderer process. For the URL with the same origin however the new request somehow clashed with the previous request in a way I cannot fully explain and navigation never finished. The fix is to ask the WebContents to resume loading instead of re-requesting it. This is also what happens in a ChromeTabbedActivity on clicking _blank links and is much faster than previous code. BUG=736995 ========== to ========== Fixes stuck navigation when opening PWA in-scope page in CCT. The issues was happening because CustomTabActivity was requesting to load the URL on the WebContents it received. URL loading was already happening at this point, as navigation was initiated in the renderer earlier. It worked for URLs with different origin, as re-requesting the URL load was causing creating new RenderFrame in a new Renderer process. For the URL with the same origin however the new request somehow clashed with the previous request in a way I cannot fully explain and navigation never finished. The fix is to ask the WebContents to resume loading instead of re-requesting it. This is also what happens in a ChromeTabbedActivity on clicking _blank links and is much faster than what I did in previous patch. BUG=736995 ==========
Description was changed from ========== Fixes stuck navigation when opening PWA in-scope page in CCT. The issues was happening because CustomTabActivity was requesting to load the URL on the WebContents it received. URL loading was already happening at this point, as navigation was initiated in the renderer earlier. It worked for URLs with different origin, as re-requesting the URL load was causing creating new RenderFrame in a new Renderer process. For the URL with the same origin however the new request somehow clashed with the previous request in a way I cannot fully explain and navigation never finished. The fix is to ask the WebContents to resume loading instead of re-requesting it. This is also what happens in a ChromeTabbedActivity on clicking _blank links and is much faster than what I did in previous patch. BUG=736995 ========== to ========== Fixes stuck navigation when opening PWA in-scope page in CCT. The issues was happening because CustomTabActivity was requesting to load the URL on the WebContents it received. URL loading was already happening at this point, as navigation was initiated in the renderer earlier. It worked for URLs with different origin, as re-requesting the URL load was causing creating new RenderFrame in a new Renderer process. For the URL with the same origin however the new request somehow clashed with the previous request in a way I cannot fully explain and navigation never finished. The fix is to ask the WebContents to resume loading instead of re-requesting it. This is also what happens in a ChromeTabbedActivity on clicking _blank links and is much faster than what I did in previous patch. BUG=736995 ==========
piotrs@chromium.org changed reviewers: + dominickn@chromium.org, yusufo@chromium.org
Hey folks, can you take a look? Yusuf as the owner of CustomTabActivity and Dominick for WebappNavigationTest.
The CQ bit was checked by piotrs@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...
test lgtm % nit https://codereview.chromium.org/2964183002/diff/1/chrome/android/javatests/sr... File chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java (right): https://codereview.chromium.org/2964183002/diff/1/chrome/android/javatests/sr... chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java:42: private static final String OTHER_PAGE_PATH = Nit: perhaps rename this variable IN_SCOPE_PAGE_PATH for clarity?
Thanks Dom! https://codereview.chromium.org/2964183002/diff/1/chrome/android/javatests/sr... File chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java (right): https://codereview.chromium.org/2964183002/diff/1/chrome/android/javatests/sr... chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java:42: private static final String OTHER_PAGE_PATH = Good idea, done!
https://codereview.chromium.org/2964183002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java (right): https://codereview.chromium.org/2964183002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java:616: mReceivedLoadingWebContents = true; webcontents.isLoading not working here?
The CQ bit was checked by piotrs@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
Thanks Yusuf https://codereview.chromium.org/2964183002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java (right): https://codereview.chromium.org/2964183002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java:616: mReceivedLoadingWebContents = true; Works, done.
lgtm
The CQ bit was checked by piotrs@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...
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 piotrs@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dominickn@chromium.org, yusufo@chromium.org Link to the patchset: https://codereview.chromium.org/2964183002/#ps80001 (title: "Merge")
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": 80001, "attempt_start_ts": 1499300135771060,
"parent_rev": "e955e728e2a1ed9225ab13d2751b07eff7d291b4", "commit_rev":
"7bf1a0ba5782af6e950604667ecf1882b189c858"}
Message was sent while issue was closed.
Description was changed from ========== Fixes stuck navigation when opening PWA in-scope page in CCT. The issues was happening because CustomTabActivity was requesting to load the URL on the WebContents it received. URL loading was already happening at this point, as navigation was initiated in the renderer earlier. It worked for URLs with different origin, as re-requesting the URL load was causing creating new RenderFrame in a new Renderer process. For the URL with the same origin however the new request somehow clashed with the previous request in a way I cannot fully explain and navigation never finished. The fix is to ask the WebContents to resume loading instead of re-requesting it. This is also what happens in a ChromeTabbedActivity on clicking _blank links and is much faster than what I did in previous patch. BUG=736995 ========== to ========== Fixes stuck navigation when opening PWA in-scope page in CCT. The issues was happening because CustomTabActivity was requesting to load the URL on the WebContents it received. URL loading was already happening at this point, as navigation was initiated in the renderer earlier. It worked for URLs with different origin, as re-requesting the URL load was causing creating new RenderFrame in a new Renderer process. For the URL with the same origin however the new request somehow clashed with the previous request in a way I cannot fully explain and navigation never finished. The fix is to ask the WebContents to resume loading instead of re-requesting it. This is also what happens in a ChromeTabbedActivity on clicking _blank links and is much faster than what I did in previous patch. BUG=736995 Review-Url: https://codereview.chromium.org/2964183002 Cr-Commit-Position: refs/heads/master@{#484407} Committed: https://chromium.googlesource.com/chromium/src/+/7bf1a0ba5782af6e950604667ecf... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/7bf1a0ba5782af6e950604667ecf... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
