Turn on PlzNavigate for webview in 62 |
||||||||||||||
Issue descriptionPer bug 702785 , Android WebView doesn't use Finch. We should probably turn it on the m62 branch now, to get bug reports from beta users.
,
Sep 14 2017
The launch bug is issue 689549. I think all TPMs know about it by now :) - in particular we have been working with amineer@ and govind@.
,
Sep 14 2017
I see. cc'ing them here, along with cmasso@ who is webview TPM.
,
Sep 14 2017
,
Sep 14 2017
,
Sep 14 2017
,
Sep 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dbc2bb9217483a8ceac23cd41bb8dcc7453c8f4b commit dbc2bb9217483a8ceac23cd41bb8dcc7453c8f4b Author: Nate Fischer <ntfschr@chromium.org> Date: Fri Sep 15 01:14:47 2017 AW: enable PlzNavigate via CLI switch This CL enables PlzNavigate (browser side navigation) by default for WebView only. This is to try out the new code path before launching at 100% for all of chrome. Bug: 765250 Test: Manual Change-Id: Iff2d0585e11075e71fe196df47503d950e2b26b3 Reviewed-on: https://chromium-review.googlesource.com/667502 Reviewed-by: Selim Gurun <sgurun@chromium.org> Commit-Queue: Nate Fischer <ntfschr@chromium.org> Cr-Commit-Position: refs/heads/master@{#502121} [modify] https://crrev.com/dbc2bb9217483a8ceac23cd41bb8dcc7453c8f4b/android_webview/lib/aw_main_delegate.cc
,
Sep 15 2017
Requesting a merge so that we can try this out on beta before it's 100% on stable. I've manually tested PlzNavigate on Gmail, Inbox, Google Play Books, GSA, WV shell, and some 3rd party apps. So far I haven't seen any major regressions.
,
Sep 15 2017
This bug requires manual review: M62 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 15 2017
I am approving this merge so we can get something out of beta channel.
,
Sep 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3ac77fd0987d08feb63ca059d582cae89a30ec88 commit 3ac77fd0987d08feb63ca059d582cae89a30ec88 Author: Tim Volodine <timvolodine@chromium.org> Date: Fri Sep 15 14:40:24 2017 Revert "AW: enable PlzNavigate via CLI switch" This reverts commit dbc2bb9217483a8ceac23cd41bb8dcc7453c8f4b. Reason for revert: speculative revert, might have broken Android Webview L/M/N bots, see crbug.com/765587 Original change's description: > AW: enable PlzNavigate via CLI switch > > This CL enables PlzNavigate (browser side navigation) by default for > WebView only. This is to try out the new code path before launching at > 100% for all of chrome. > > Bug: 765250 > Test: Manual > Change-Id: Iff2d0585e11075e71fe196df47503d950e2b26b3 > Reviewed-on: https://chromium-review.googlesource.com/667502 > Reviewed-by: Selim Gurun <sgurun@chromium.org> > Commit-Queue: Nate Fischer <ntfschr@chromium.org> > Cr-Commit-Position: refs/heads/master@{#502121} TBR=sgurun@chromium.org,ntfschr@chromium.org Change-Id: Ia07d03630a71b0c1beabeee4faf5ce685131bf70 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 765250 Reviewed-on: https://chromium-review.googlesource.com/667150 Reviewed-by: Tim Volodine <timvolodine@chromium.org> Commit-Queue: Tim Volodine <timvolodine@chromium.org> Cr-Commit-Position: refs/heads/master@{#502249} [modify] https://crrev.com/3ac77fd0987d08feb63ca059d582cae89a30ec88/android_webview/lib/aw_main_delegate.cc
,
Sep 15 2017
A summary of meeting between webview leads and jam@ about this (sorry cmasso@, we missed you there): 1) Webview team is ok with enabling this on M62 this time because a) finch isn't there b) too little beta/dev/canary channel users anyways. 2) We should be able to disable the feature on M63. (or M64? cmasso@ can tell) 3) changwan@ will work with brettw@ and cmasso@ to update the launch process template to note about the peculiarities about webview launch in order to avoid this type of confusion. cmasso@, I think release merge should always be accompanied by CTS tests at all times. Could we update the merge process? Thanks.
,
Sep 15 2017
Hey changwan@, sorry I received the meeting invite late to make it on time. The plan sounds good and I will take a closer look at 3) to understand what could be improved. I am not sure I understand you comment about release merge and CTS tests :) Please elaborate a little more? we could also just chat if you are available.
,
Sep 15 2017
cmasso@, Since we are planning to release as part of M62 (if everything looks good), I think we should mark as RBS to be in the radar? Thanks!
,
Sep 15 2017
+RBS (this doesn't have to block betas, but it should get in the earliest beta reasonable). clamy@ already enabled PlzNavigate on trunk (0dbded05ad0ba1c3d8388fd649130509ab97f07a), so there's no sense in relanding this CL on trunk. Instead, I'm just going to merge the CL back to M62 once issue 765587 is resolved & merged back. Please holler if anyone objects.
,
Sep 15 2017
,
Sep 18 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/267e445a8ff514d5169b4727f583ef3a8988b7fd commit 267e445a8ff514d5169b4727f583ef3a8988b7fd Author: Nate Fischer <ntfschr@chromium.org> Date: Mon Sep 18 19:41:16 2017 AW: enable PlzNavigate via CLI switch This CL enables PlzNavigate (browser side navigation) by default for WebView only. This is to try out the new code path before launching at 100% for all of chrome. TBR=ntfschr@chromium.org (cherry picked from commit dbc2bb9217483a8ceac23cd41bb8dcc7453c8f4b) Bug: 765250 Test: Manual Change-Id: Iff2d0585e11075e71fe196df47503d950e2b26b3 Reviewed-on: https://chromium-review.googlesource.com/667502 Reviewed-by: Selim Gurun <sgurun@chromium.org> Commit-Queue: Nate Fischer <ntfschr@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#502121} Reviewed-on: https://chromium-review.googlesource.com/671187 Reviewed-by: Nate Fischer <ntfschr@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#300} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/267e445a8ff514d5169b4727f583ef3a8988b7fd/android_webview/lib/aw_main_delegate.cc
,
Sep 18 2017
Ok, PlzNavigate should now be enabled for the next WebView beta. Current plan is to revert this before M62 stable ( issue 766257 ), so expect to see the revert land against this bug. --- To reiterate on the plan, if anyone sees navigation-related bugs, *please* file them against me so I can triage them (tracking bug: issue 766255). Even though we plan to not launch in M62 stable, it's still critical to know about these bugs for future launch.
,
Sep 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/332b11b2433d815d9bcb387acc5247ac588af755 commit 332b11b2433d815d9bcb387acc5247ac588af755 Author: Nate Fischer <ntfschr@chromium.org> Date: Tue Sep 19 03:38:24 2017 Revert "AW: enable PlzNavigate via CLI switch" This reverts commit 267e445a8ff514d5169b4727f583ef3a8988b7fd. Reason for revert: We've already seen lots of breakages with PlzNavigate Original change's description: > AW: enable PlzNavigate via CLI switch > > This CL enables PlzNavigate (browser side navigation) by default for > WebView only. This is to try out the new code path before launching at > 100% for all of chrome. > > TBR=ntfschr@chromium.org > > (cherry picked from commit dbc2bb9217483a8ceac23cd41bb8dcc7453c8f4b) > > Bug: 765250 > Test: Manual > Change-Id: Iff2d0585e11075e71fe196df47503d950e2b26b3 > Reviewed-on: https://chromium-review.googlesource.com/667502 > Reviewed-by: Selim Gurun <sgurun@chromium.org> > Commit-Queue: Nate Fischer <ntfschr@chromium.org> > Cr-Original-Commit-Position: refs/heads/master@{#502121} > Reviewed-on: https://chromium-review.googlesource.com/671187 > Reviewed-by: Nate Fischer <ntfschr@chromium.org> > Cr-Commit-Position: refs/branch-heads/3202@{#300} > Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} TBR=sgurun@chromium.org,ntfschr@chromium.org Change-Id: I173db64902172655a72161a244f3923c9c9ef196 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 765250 Reviewed-on: https://chromium-review.googlesource.com/672156 Reviewed-by: Nate Fischer <ntfschr@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#316} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/332b11b2433d815d9bcb387acc5247ac588af755/android_webview/lib/aw_main_delegate.cc
,
Sep 19 2017
Reverted due to the bugs we've seen. The plan now is to fix the known bugs and reland in time for a future beta.
,
Sep 27 2017
,
Oct 10 2017
ntfschr@, Just wondering the above change has been reverted from M62 branch (3202) as well? Since we are promoting M62 to Stable soon. Also please tag the respective milestone for future tracking. Thank you!
,
Oct 10 2017
The merge has indeed been reverted (comment #10), which is what we want. The feature is already shipped in M63, so there's no more work for this bug. |
||||||||||||||
►
Sign in to add a comment |
||||||||||||||
Comment 1 by changwan@chromium.org
, Sep 14 2017