New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 778044 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug
Proj-Servicification

Blocking:
issue 603396



Sign in to add a comment

Enable LoadingWithMojo on 63 branch for Android WebView

Project Member Reported by jam@chromium.org, Oct 24 2017

Issue description

(filing bug to track what we discussed in today's network service meeting)

Android WebView doesn't use Finch. So mojo loading isn't enabled there for it. Since we're now doing trials on m62 stable on Android, we can't delete the code on trunk until webview is switched. It's unfortunately too late to do this for m62 on webview, so we should aim for 63.

Thanks Yuzhu for pointing this out!

I believe it should already be enabled for chromecast, but we should double check as well.
 
Components: Mobile>WebView
Let's try to get this into M64 dev & M63 beta (unless there are objections). A couple questions:

 1) Is this enabled yet on trunk? If not, which bug can I follow for this?
 2) Is there a flag for this?
It's enabled by default on platforms other than Android. It's disabled on Android including WebView. The flag is LoadingWithMojo. The tracking issues are  issue 603396  and issue 703483.

Comment 3 by boliu@chromium.org, Oct 25 2017

Labels: OS-Android
What's the state of the finch experiment for m62 chrome right now?

Comment 4 by jam@chromium.org, Oct 25 2017

Cc: kenjibaheux@chromium.org
Right now it's at 10% enabled, 10% control, and 80% default (i.e. disabled)
https://uma.googleplex.com/p/chrome/variations?sid=c1bf26e992efdb7ccd8d519c6540a8ef

The crashes on 62 look similar to without the experiment, so I would suspect we would ramp this up to 100%.
Can someone send the CL to append the flag for WebView? Example CL: https://chromium-review.googlesource.com/c/chromium/src/+/667502

Comment 6 by boliu@chromium.org, Oct 25 2017

Sounds like this will get enabled by default for android on trunk real soon. Can probably just wait for that

63 is also the first release with plznavigate. Not sure if there is value in waiting for a 63 beta to go out with plznavigate only first before enabling this on branch

Comment 7 by jam@chromium.org, Oct 25 2017

@boliu: plznavigate and mojoloading are orthogonal.
Turning mojoloading on for m63, which should be pretty safe at this point, allows us to remove the old code one release earlier.

Comment 8 by boliu@chromium.org, Oct 25 2017

I just meant one build of m63. Like .1 has plznavigate only so we get a week of feedback with plznavigate only. Then .2 can have mojoloading as well.

Comment 9 by jam@chromium.org, Oct 25 2017

@boliu: per Claude, the first 63 beta build will be cut in 2 hours, so you have your wish :) I'll send a cl to turn this on for webview.
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6917671eb785e39d526dc11f1a9c4cd669669d95

commit 6917671eb785e39d526dc11f1a9c4cd669669d95
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Oct 26 00:19:27 2017

Enable LoadingWithMojo for webview since it doesn't use Finch.

BUG= 778044 

Change-Id: Id6a82f4ee86c30dd8045a5ebea527f44ae3639fd
Reviewed-on: https://chromium-review.googlesource.com/738831
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511657}
[modify] https://crrev.com/6917671eb785e39d526dc11f1a9c4cd669669d95/android_webview/lib/aw_main_delegate.cc

Comment 11 by jam@chromium.org, Oct 26 2017

Labels: Merge-Request-63
Project Member

Comment 12 by sheriffbot@chromium.org, Oct 26 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-63 Merge-Approved-63
All good! 
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 27 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f1028ee5d1d96b381d237f9d2e4d2dc5f54c2a26

commit f1028ee5d1d96b381d237f9d2e4d2dc5f54c2a26
Author: John Abd-El-Malek <jam@chromium.org>
Date: Fri Oct 27 19:40:58 2017

Enable LoadingWithMojo for webview since it doesn't use Finch.

BUG= 778044 

Change-Id: Id6a82f4ee86c30dd8045a5ebea527f44ae3639fd
Reviewed-on: https://chromium-review.googlesource.com/738831
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#511657}(cherry picked from commit 6917671eb785e39d526dc11f1a9c4cd669669d95)
Reviewed-on: https://chromium-review.googlesource.com/742301
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#270}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/f1028ee5d1d96b381d237f9d2e4d2dc5f54c2a26/android_webview/lib/aw_main_delegate.cc

Comment 15 by jam@chromium.org, Oct 27 2017

Cc: yhirano@chromium.org
Owner: jam@chromium.org
Status: Fixed (was: Assigned)
Components: Internals>Network>Service
Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.

Sign in to add a comment