Tested the issue in WebView and could not reproduce the issue.

Steps Followed:
1. Enabled WebView #63.0.3239.31
2. Navigated to stock email app
3. Able to reply the received emails

Chrome versions tested:
63.0.3239.31

OS
Android 6.0.1

Android Devices
6.0.1; SM- G920I Build/MMB29K

@stephen: Could you please help us with the details of your device and if possible a sample screencast for further triaging of the issue.

Thanks!!

Here is a link to a screen AST of the issue. None of the menu options work
when the message is being loaded, so I have to kill the app and later find
that draft reply had been created (but not sent).

https://youtu.be/LmjjIK7_O7w

Below is info abt my phone. Not sure if it contains all you want. Let me
know specifically what you need.

All Info
----------------------
Battery Info
----------------------
Level: 51
Scale: 100
Temperature: 29.0C, 109.80F
Technology: Li-ion
Voltage: 3V
Battery present: true
Status: discharging
Plugged status: not plugged in
Health: good

Build Info
----------
Board: APQ8084
Boot Loader: N910VVRU2CQI2
Brand: Verizon
CPU_ABI: armeabi-v7a
CPU_ABI2: armeabi
Device: trltevzw
Display: MMB29M.N910VVRU2CQI2
Fingerprint:
Verizon/trltevzw/trltevzw:6.0.1/MMB29M/N910VVRU2CQI2:user/release-keys
Hardware: qcom
Host: SWDD6223
Id: MMB29M
Manufacturer: samsung
Model: SM-N910V
Product: trltevzw
Radio: unknown
Serial: 15566404
Tags: release-keys
Time: 1504525620000
User: dpi

Build Version
Codename: REL
Incremental: N910VVRU2CQI2
Release: 6.0.1
Sdk: 23
SDK_INT: 23

Config Info
-----------
Font Scale: 1.09
Locale: en_US
Mobile Country Code: 311
Mobile Network Code: 480
Keyboard: None
Keyboard hidden: NO
Hard keyboard hidden: NO
Available navigation: None
Is navigation hidden: YES
Orientation: Portrait
Screen layout: large long
Touch screen: Finger
UI Mode: normal not night mode

File System Info
----------------------
External Storage Size: 23.34GB
Available (user): 8.58GB
Available (all): 8.58GB
External storage directory: /storage/emulated/0
Storage state: mounted
Data directory: /data
Root directory: /system
Download cache directory: /cache

System Properties Info
----------------------
http.agent: Dalvik/2.1.0 (Linux; U; Android 6.0.1; SM-N910V Build/MMB29M)
java.io.tmpdir: /data/user/0/org.reassembler.textinfo/cache
user.home:


Steve
Stephen Landau
crossculture.srlandau.com
saffronrobes.srlandau.com
templeboys.srlandau.com
Sent via Wireless Smartphone

Thank you for providing more feedback. Adding requester "sandeepkumars@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Requesting MTV team to check the issue on SM-N910V 6.0.1 as it's not available with Hyd team.

Thanks!!

I have updated the WebView Beta 63.0.32.39.53 of Nov 16, 2017. This does NOT resolve this issue. Have had to remove beta WebView and revert to installed base app in order to use stock email app. I will review future updates.

If our test-team can reproduce this it would be great with a bisect. 

dneelamegam@ checking it, he will update soon.

I was able to repro this issue on Samsung Note 4/SM-N910T3/LMY47X (T-Mobile)

Good build: 63.0.3216.3
Bad Build: 63.0.3217.0
unfortunately our CL bisect script don't work for OEM devices

https://chromium.googlesource.com/chromium/src/+log/63.0.3216.3..63.0.3217.0?pretty=fuller&n=10000

For what it's worth I'm experiencing this on a Verizon Note 4 Manufacturer: samsung
Model: SM-N910V (Verizon).

Glad that the issue could be reproduced. Will this now be fixed?

Thank you.

This contains PlzNavigate in the git range, perhaps that could be to blame?

Test team - in order to further investigate, I need a samsung device which can repro. Strong preference for either userdebug or >= N. Do we have any such devices?

we have a few userdebug samsung devices, they should be under clankdevices right now with a note

Unfortunately, both our userdebug samsung note 4s are stuck at kitkat. While this might be a real bug, I don't think we can proceed without being able to repro (and install webview builds).

stephen.landau@ any chance this has been fixed by a more recent webview release?

Since I don't see a way forward, I'm closing this as wontfix.

Quite amazing to me that Google can't scare up a platform to repro and fix
this bug.

FYI: latest update of WebView
63.0.3239.107, December 13, 2017
does not fix the bug.

Steve
Stephen Landau
crossculture.srlandau.com
saffronrobes.srlandau.com
templeboys.srlandau.com
Sent via Wireless Smartphone

For security reasons, there's very little developers can do with user devices (as opposed to userdebug builds). That's a good thing--it ensures users only run trusted code.

This bug appears to only repro on specific samsung phones, which makes it very difficult for us to obtain a userdebug image (this is easier for nexus/pixel devices). I agree this is unfortunate. If other users have similar issues, we can try to revisit this.

...and therein lies the beauty and the bane of the Android (as opposed to
the Apple model). What this perhaps says is that to have the best of all
worlds better stick with Google's Pixel devices.

Steve
Stephen Landau
crossculture.srlandau.com
saffronrobes.srlandau.com
templeboys.srlandau.com
Sent via Wireless Smartphone

If we need different OS versions with userdebug for the samsung devices we have, we should just be asking samsung for them (they won't be updated OTA, as samsung's public OTA servers won't provide userdebug images - but they're not "stuck" on any particular OS version unless samsung no longer provide new OS versions for that device).

Does this mean that you will ask Samsung if they can provide a userdebug image for the device in question, and if they provide you with one you will attempt to resolve this bug?

 

Thank you.

 

Steve
Stephen Landau
 <http://crossculture.srlandau.com/> Middle Way Multimedia Publishing
 <http://saffronrobes.srlandau.com/> saffronrobes.srlandau.com
 <http://templeboys.srlandau.com/> templeboys.srlandau.com 

 

From: to… via monorail [mailto:monorail+v2.722631275@chromium.org] 
Sent: Thursday, December 14, 2017 11:44 AM
To: stephen.landau@gmail.com
Subject: Issue 781535 in chromium: When replying to email using stock email app

We are in contact with Samsung on what we believe to be the same problem (internal bug report: http://b/70696775). I'll update this thread if we get any progress there.

Based on what I've seen on the buganizer (http://b/70696775, https://issuetracker.google.com/70701659, http://b/70777395), I'm more confident this is PlzNavigate.

I'll reopen this, but there's very little we can do to debug this right now. One of the buganizer issues indicates this may be caused by WebView no longer calling onPageFinished(). Without knowing what URLs are being loaded, there's very little we can do.

---

For what it's worth, I looked at gmail (which does not repro). Gmail loads data URls with loadDataWithBaseUrl (and both onPageStarted and onPageFinished are called for "about:blank"). Maybe Samsung OEM mail is also loading data URLs, but is using the webview in some unexpected way, causing onPageFinished to not be called.

Got the same bug on Samsung SM-G361F with Android 5.1.1. Even the current beta 64.0.3282.29 does not fix it. 

Issue summary: when replying or forwarding an email with the stock Email app the device remains stuck with a pop-up saying "Loading e-mail...". Pressing the Back button few times brings you back to the compose window but nothing can be typed-in.

I can provide the app emaillog.txt debug trace if necessary. We concluded it is a problem with Webview by comparing two devices. FWIW, the last javascript calls are for file:///android_asset/htmleditor/src/SelectionController.js, which I have no idea to which app it belongs to.

This issue blocks most Italian Public Administration smartphones obtained through a nationwide tender. See angry comments on the Play Store.

> current beta 64.0.3282.29 does not fix it

Correct. We have not fixed this issue, but we are actively investigating (still in conversation with Samsung).

> blocks most Italian Public Administration smartphones

Is that because of the samsung OEM mail app, or are you referring to some different app?

Il 04/Gen/2018 21:24, "ntfs… via monorail" <
monorail+v2.1602762116@chromium.org> ha scritto:

Correct. We have not fixed this issue, but we are actively investigating
(still in conversation with Samsung).


Thanks for the update. Much appreciated.

Is that because of the samsung OEM mail app, or are you referring to some
different app?


OEM mail app. At least in my Company/Organization.

I will wait for updates on this thread.

I had reported this same issue on my Samsung Galaxy Note 4, and this is the
application that exhibits the problem on my phone (see image attached).

Steve
Stephen Landau
crossculture.srlandau.com
saffronrobes.srlandau.com
templeboys.srlandau.com
Sent via Wireless Smartphone

stephen.landau@ I can't see any attachment. I've attached a screenshot circling the app icon--let me know if that looks right.

This indeed has the package name "com.android.email", but I'm pretty sure this is Samsung's app, not something Android produces.

Deleted: samsung-email-app.png 424 KB

	samsung-email-app.png 424 KB View Download

Sorry, I can't see your attachment. Maybe the attachments are being
stripped out.

I think it is Samsung's email app, although nowhere does the word "samsung"
appear associated with it.

This is the icon associated with the email app:
https://www.google.com/search?q=email+icon+on+android&client=ms-android-verizon&prmd=ivn&source=lnms&tbm=isch&sa=X&ved=0ahUKEwjz2IGozr_YAhWCdSYKHbaaDG4Q_AUIESgB&biw=360&bih=560#imgrc=qKdCb8LPB-Au5M
:

It is listed on my Note 4 as:
com.android.email
Version: 5.0.0.0400

Steve
Stephen Landau
crossculture.srlandau.com
saffronrobes.srlandau.com
templeboys.srlandau.com
Sent via Wireless Smartphone

On Jan 4, 2018 7:03 PM, "ntfs… via monorail" <monorail+v2.1602762116@
chromium.org> wrote:

> Sorry, I can't see your attachment. Maybe the attachments are being stripped out.

I think you need to view the actual web page for this bug. But regardless, we're talking about the same app. Thanks for confirming (version number and icon match).

Update: we believe we understand the underlying issue for the Samsung app, however we still need to investigate how to fix it.

We accidentally introduced a behavior change when we have multiple concurrent loads (e.g. >=3 overlapping loads):

  wv.loadUrl(url1);
  wv.loadUrl(url2);
  wv.loadUrl(url3);
  // we have problems if we have 3+ at the same time, without waiting for them to finish...

Before PlzNavigate, we would emit onPageStarted/Finished for each of those loads--with PlzNavigate, we only observe onPageStarted/Finished for the first & last loads.

I see that we treat the first and non-first loads differently here [1]:

  if (should_dispatch_beforeunload && !IsRendererDebugURL(dest_url)) {
    // We take this branch on the non-first loads
    navigation_request->SetWaitingForRendererResponse();
    frame_tree_node->current_frame_host()->DispatchBeforeUnload(
        true, reload_type != ReloadType::NONE);
  } else {
    // We take this branch on the first load
    navigation_request->BeginNavigation();
  }

The else-branch creates a NavigationHandle object (its destructor causes onPageFinished()). The if-branch doesn't create this object, which explains why we're missing onPageFinished for the other loads. The final load also emits onPageFinished, but through a different stack (via RenderFrameHostImpl::OnBeforeUnloadACK).

I'm still digging into this, to understand DispatchBeforeUnload and see if we're doing the right thing.

---

I'm adjusting target milestone to M64--we probably can't let this stay broken until M65.

[1] https://cs.chromium.org/chromium/src/content/browser/frame_host/navigator_impl.cc?l=1058

Ok I think I see what is happening with BeforeUnload. On the first navigation, we're in a brand new unintialized tab so we don't need to check for BeforeUnload because we don't have a document yet. However, when we start the navigation we initialize the current RenderFrameHost. For subsequent navigations, we have an initialized RenderFrameHost, and we think we should have it execute BeforeUnload before starting the navigation. Provided the initial document of a tab doesn't have a BeforeUnload event registered, it shouldn't be necessary to execute BeforeUnload in that case (but it might be hard to figure this out from the browser process without going to the renderer process).

That said the navigation code does not support concurrent loads (and it has always been the case). We generally only keep the last one the user initiated. There are slight differences between pre and post PlzNavigate there, but 10 concurrent navigations would never have succeeded even without PlzNavigate.

> but 10 concurrent navigations would never have succeeded even without PlzNavigate.

Right, makes sense.

> Provided the initial document of a tab doesn't have a BeforeUnload event registered, it shouldn't be necessary to execute BeforeUnload in that case

Is it feasible to emit onPageStarted/onPageFinished from the BeforeUnload branch too? The bug appears fixed if I hard-code to always take the DispatchBeforeUnload branch.

Er, hard-code to always take the "BeginNavigation" branch.

From the navigation code point of view, the navigation hasn't started until Beforeunload executes (the user can choose not to let the navigation start in a dialog during that event). The way we send events mirrors that: after BeforeUnload has executed we create a NavigationHandle. onPageStarted/onPageFinished comes from the fact that we have a NavigationHandle.

There is some amount of code in Chrome that relies on the NavigationHandle being created at the time it is created: for example PageLoadMetrics want the start time of navigation from OnNavigationStarted, and this time is after BeforeUnload executed.

As explained above, we might be able to skip executing BeforeUnload if the frame hasn't committed any real load. However, this might break embedder code if their default document has a BeforeUnload event and they rely on it - which is a possibility.

I think we should look more into exactly what sequence of navigations the app is doing and why (I asked for more info on the internal bug). The only way I can see this making any real sense is if the app is loading javascript: URLs in sequence to evaluate a bunch of JS - if it's loading actual content then I'm very skeptical that this ever worked reliably for arbitrary numbers of loads and it was probably only working by fluke of timing and implementation.

If we can, I'd much rather try to stick with being able to say "if you expect navigations to actually happen, wait for the callbacks to tell you they have happened before you start the next one" - even if we have to put some specific hack in to unbreak this email app, I'm not sure we generally want to guarantee "every call to loadUrl will definitely result in onPageStarted/onPageFinished, no matter what", or that this is the only case in which that isn't currently true..

@torne: JavaScript URLs are handled differently and should not fire any onPageStarted/onPageStopped notification at all as they're not real navigations. We do not expose their load outside of content/. In particular, we would never wait for BeforeUnload to execute when getting a navigation to a JavaScript URL.

Ah, okay. In which case, it's extremely dubious why this case *should* even be working at all :)

 Issue 801149  has been merged into this issue.

They were mostly doing loadDataWithBaseUrl() and I think a loadUrl(). I'm also pretty confused as to what they were really trying to do (their sample app didn't reveal the real intent).

If I can get a debuggable samsung (still waiting), I can possibly fake the missing callbacks for Samsung OEM mail only. If it really is a matter of missing callbacks (and WebView is doing the same "real work") then the hack might work.

I have a hacky workaround in https://chromium-review.googlesource.com/c/chromium/src/+/863463. Feels too rough to submit--I think we really need to change navigation code to emit the events when we trigger the BeforeUnload branch.

Let me know if there's a better spot (either in android_webview/ or content/) to address this.

Also, I still don't know if this fixes the app--can't get a debuggable samsung image.

> even if we have to put some specific hack in to unbreak this email app

Do we have much choice here? Does Samsung have some way to distribute app updates (it's not on the play store)?

Thank you all for the effort put into this issue.

As an affected user I can contribute with a physical Samsung device to run
debug and tests (locally), I have the mail application apk and I can run a
dev version of the Webview.

That's certainly outside of all your workflows, I know.

Get in touch if I can help.

Yes, sorry if I wasn't clear: I'm assuming we'll have to put in some hack to unbreak this app because past experience suggests this class of app isn't updateable.

What I'm not sure about is whether we want to make a general change to webview behaviour here. This really doesn't seem like a sensible thing to support in general. I'd probably be in favour of making this a specific quirk flag that we only turn on in particular versions of particular known-broken apps, the same way we handle various IME bugs: see  https://cs.chromium.org/chromium/src/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java?rcl=01b5e14bfd1f1f948b294db5f7f486f276a157d0&l=589 for an example.

ntfschr@ we are few days away from releasing M64. Even if we had a fix for this issue now, I will be very hesitant merging it into M64 at this point. Can this be punted to M65?

I guess we couldn't get this resolved for M64, I'll try to get something for M65.

 Issue 804215  has been merged into this issue.

 Issue 803658  has been merged into this issue.

Update:

I received a debuggable Samsung device. I confirm my hack workaround [1] seems to let reply/forward work again (with minimal manual testing).

This workaround assumes that Samsung was relying on buggy behavior. I need to investigate further to verify this, and to fully understand what behavior Samsung really expected. After that, we can assess if a workaround really is the right approach.

---

Unfortunately, 65 is the earliest we might expect a fix/workaround.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/863463

Copied from https://bugs.chromium.org/p/chromium/issues/detail?id=804208

THIS TEMPLATE IS FOR FILING BUGS ON THE ANDROID SYSTEM WEBVIEW. GENERAL WEB
BUGS SHOULD BE FILED USING A DIFFERENT TEMPLATE!

Device name: Samsung Galaxy S4 Value Edition 
Android version: 5.01
WebView version (from system settings -> Apps -> Android System WebView): 64.0.3282.99
Application: E-mail (Samsung)
Application version: 5.0.0.0200

URLs (if applicable):



Steps to reproduce:
(1) open e-mail
(2) open message
(3) press forward button

Expected result: show new message window with new input fields for recipient address 


Actual result: new message window opens but a progress bar pop's up on top of it showing "loading message" and it never finishes. This prevents you from entering the recipient address. You can use the back button to abort the progress dialog so that is dissappears but that doesn't enable you to send the message. It just will not work. Rolling back to version android system webview version 63.0.3239.107 or earlier is not possible. I believe this bug was intoduced with version 63.0.3239.111. Beta versions after this version also have this bug.

Hereby a screenshot of the progress window that pop's up when trying to forward a message.

Deleted: Screenshot_2018-01-23-10-05-43.png 144 KB

	Screenshot_2018-01-23-10-05-43.png 144 KB View Download

Last I checked, this worked for me (originally suggested here [1] in the 3rd comment):

1. Open email, click "reply" or "forward"
2. Click the back button to dismiss the dialog
3. Type email, save as draft
4. Go into drafts mail folder
5. Open email, press "send"

[1] https://androidforums.com/threads/email-shows-loading-messages-when-i-try-to-reply-to-an-email.1225348/

 Issue 804208  has been merged into this issue.

this issue still repro on M65/65.0.3325.53 and M66/66.0.3342.3 on Samsung S5.

 Issue 808327  has been merged into this issue.

Still reproducible on 65.0.3325.53 lollipop 5 samsung galaxy s4 ve

Unfortunately, our original analysis was misguided. Here's a summary of what's actually going on, I hope someone from navigation team can help pin this down:

# What the app does:

They have 2 WebViews. WebView A loads a file:// URL, WebView B loads a file:// URL followed shortly after by loadDataWithBaseUrl().

# What the app expects:

They keep the spinner up until they receive onPageFinished for WebView A and onPageFinished for WebView B (it can be any URL, they just wait until each WebView gets its first onPageFinished).

# What WebView does (post-PlzNavigate):

WebView A has no issues (it emits onPageFinished as expected), but WebView B is broken. I would expect WebView B to emit 2 onPageFinished calls (at the very least, onPageFinished for the baseUrl), but it emits neither. This sounds like a bug.

# Things I've tried which get rid of the bug:

 - delay the loadDataWithBaseUrl() call until loadUrl(file://) is done -> the bug only repros with overlapping ("concurrent") loads
 - remove the baseUrl (which reduces this to a loadData() call) -> the bug is specific to loadDataWithBaseUrl, not affecting other types of loads

Are there any special cases for loadDataWithBaseUrl() to cancel a previous navigation? My original thought was that this might be mistaken for a reload (hence why we see neither the onPageFinished for the file:// URL nor the onPageFinished for the base URL). It's not obvious to me if my hypothesis is wrong, or if there's a more likely culprit.

Full investigation is on this doc [1] (googlers only).

[1] https://docs.google.com/document/d/1drJZFx0MiUO8i7CLLIZViCMRf6FFl7N-blTgSnhAh1s/edit?usp=sharing

I read through the doc and ugh this seems super hard :|

I realise we had issues with the repro app before not really doing what the email app actually does, but it seems like it might be valuable here to try to make an app that reproduces what we think is the issue right now, and minimise it as much as possible? (for example, does having two webviews have any actual relevance or does it break similarly with just the second one)

ntfschr@ any update here? M65 release is nearing.

I think our best plan of action is:

1. Land a workaround, cherry pick this back to M65 (http://crrev/c/924115 is low-risk). This helps users as quickly as possible.
2. After landing, continue investigation to see if there's a real bug or if the workaround is appropriate long-term. If it's our bug, apply the fix on 66 before branch and remove the workaround.

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

commit da8150a61216a0eaa65428613d375968bec2fecc
Author: Nate Fischer <ntfschr@chromium.org>
Date: Fri Feb 16 22:14:41 2018

AW: workaround Samsung mail app issue

This applies a workaround for one of Samsung's OEM mail apps which was
negatively affected by PlzNavigate. This is meant as a minimal temporary
workaround until we finish investigation on the issue.

The app is loading 2 URLs (a file:// URL followed by
loadDataWithBaseUrl) in quick succession. This workaround posts the
second load by 200ms. In my testing, I've found that 40ms is more than
enough, so 200ms should be very safe (but still fast enough to not
adversely impact user experience).

Bug: 781535
Test: manual - with the actual Samsung mail app
Change-Id: I1fdb002c377af083158a5dc4e0e7ce067a176d30
Reviewed-on: https://chromium-review.googlesource.com/924115
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537428}
[modify] https://crrev.com/da8150a61216a0eaa65428613d375968bec2fecc/android_webview/java/src/org/chromium/android_webview/AwContents.java

Requesting the merge now, but we'll let this bake in canary until Tuesday before cherry picking.

This bug requires manual review: M65 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), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

How are things looking in Canary Nate? is this fix safe enough for M65 at this point in the release cycle?

There hasn't been a canary release since this landed.

There will be one today.

cmasso@ workaround verified on 66.0.3352.3, with both reply/forward. Ready for merge review.

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

commit 81ac2d344afd7f52e36966dc513c1aa3b19c8f73
Author: Nate Fischer <ntfschr@chromium.org>
Date: Fri Feb 23 20:01:00 2018

AW: workaround Samsung mail app issue

This applies a workaround for one of Samsung's OEM mail apps which was
negatively affected by PlzNavigate. This is meant as a minimal temporary
workaround until we finish investigation on the issue.

The app is loading 2 URLs (a file:// URL followed by
loadDataWithBaseUrl) in quick succession. This workaround posts the
second load by 200ms. In my testing, I've found that 40ms is more than
enough, so 200ms should be very safe (but still fast enough to not
adversely impact user experience).

Bug: 781535
Test: manual - with the actual Samsung mail app
Change-Id: I1fdb002c377af083158a5dc4e0e7ce067a176d30
Reviewed-on: https://chromium-review.googlesource.com/924115
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#537428}(cherry picked from commit da8150a61216a0eaa65428613d375968bec2fecc)
Reviewed-on: https://chromium-review.googlesource.com/935401
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#577}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/81ac2d344afd7f52e36966dc513c1aa3b19c8f73/android_webview/java/src/org/chromium/android_webview/AwContents.java

Workaround has landed, but I'm going to keep this open for further investigation into the root cause.

If users on this thread (stephen.landau@ and others) are still interested, it would be great if you could switch to WebView beta and tell us if the release next week clears things up for you (this missed 65.0.3325.85, so the release after that). Also, thank you for your patience on this issue.

I am on WebView beta and will test when the release is available.

Thank you.

Steve
Stephen Landau
crossculture.srlandau.com
saffronrobes.srlandau.com
templeboys.srlandau.com
Sent via Wireless Smartphone

verified the fix on M65/65.0.3325.101 and M66/66.0.3355.0 webview on 
device: samsung/trltetmofrp/trltetmo:5.1.1/LMY47X/N910T3UVU1DOK2:user/release-keys

Workaround has been verified by QA, so I'm going to remove RBS and adjust the milestone for a better fix and more complete investigation.

I believe the most recent beta release should contain the workaround. Users: if you have version 65.0.3325.109 or higher and still see this issue, please let me know ASAP.

fixed confirmed in beta 65.0.3325.109 (samsung galaxy note 3)
thanks

I've tested with the WV beta release 65.0.3325.109 of Feb 28th. This seems
to have resolved the "reply to" and "forward to" issues.

Thank you.

Steve
Stephen Landau
crossculture.srlandau.com
saffronrobes.srlandau.com
templeboys.srlandau.com
Sent via Wireless Smartphone

I've also tested samsung A7 and Note 3 with the beta release 65.0.3325.109 of Feb 28th. This resolved the "reply to" and "forward to" issues for me.
Thnx

Technical update:

As mentioned before, the root cause of this issue is that Samsung's app loads 2 URLs concurrently (loads one, then loads the other right after), and WebView does not emit onPageFinished for either URL. We're missing 2 onPageFinisheds, and there's a different cause for each.

The first onPageFinished should come when we abort the navigation to URL 1 in favor of the navigation to URL 2. My investigation in comment #30 was close: we should abort the first navigation [1], but sometimes the previous navigation_request_ is NULL (appears to be a race condition).

The second missing onPageFinished is because of loadDataWithBaseURL-specific bug. WebView uses 2 events from the content layer to determine when to emit onPageFinished: WebContentsObserver::DidFinishLoad and WebContentsObserver::DidStopLoading. We only call onPageFinished if both events are called with the same URL. Content layer consistently calls DidFinishLoad with the baseUrl (correct), but sometimes calls DidStopLoading with the data URL (wrong). There's a code path [2] which copies a NavigationEntry, but forgot to copy the baseURL attribute.

---

The fix for the second onPageFinished is trivial, so I'll upload a CL. Fortunately, this actually satisfies Samsung's expectation: the app expects at least 1 onPageFinished, for any URL.

I'll continue investigating the first missing onPageFinished and fix that separately. This is lower priority.

[1] https://cs.chromium.org/chromium/src/content/browser/frame_host/frame_tree_node.cc?q=f:frame_tree_node.cc+ERR_ABORTED&sq=package:chromium&l=486
[2] https://cs.chromium.org/chromium/src/content/browser/frame_host/navigation_controller_impl.cc?l=1275

My first try at the second missing onPageFinished is at http://crrev/c/963848. It looks like this triggers some test failures: the tests seem to explicitly expect that the base_url attribute is not carried over for cases such as navigating from a LoadDataWithBaseUrl to another page (see [1]). It seems like this code path is triggered when navigating *away* from a page with a baseUrl.

My current guess is that it's invalid for a loadDataWithBaseUrl to actually trigger this code path, and it only does so because of a bug related to us not canceling the first page load (which is the root cause of the first missing onPageFinished). I will try to confirm this guess tomorrow.

[1] https://cs.chromium.org/chromium/src/content/browser/frame_host/navigation_controller_impl_browsertest.cc?type=cs&q=NavigateFromLoadDataWithBaseURL&sq=package:chromium&l=333

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

commit ca3b1cc230867486edfb16db2718ebdb63650aaf
Author: Nate Fischer <ntfschr@chromium.org>
Date: Thu Apr 05 01:18:18 2018

AW: extend Samsung workaround for other mail app

This relaxes the check for the Samsung workaround landed in
http://crrev/c/924115, so that the workaround applies for Samsung's
other mail app which also reproduces this behavior.

Although the other mail app is referred to as
"com.samsung.android.email.provider" in our communication with them, its
implementation is actually split among 5 different APKs, and it is
"*.composer" which actually exhibits the bug.

As it turns out, the bug reproduces under the exact same sequence of
URL navigations as the other app (a file:// URL followed by
loadDataWithBaseURL("email://")).

As with http://crrev/c/924115, this is also meant as a temporary
workaround.

Bug: 781535
Test: Use both Samsung apps, test the 3 repro conditions listed in b/70696775.
Change-Id: Ic897a33dc52b78e83eaada5b4b66c555a0eb3679
Reviewed-on: https://chromium-review.googlesource.com/981691
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548280}
[modify] https://crrev.com/ca3b1cc230867486edfb16db2718ebdb63650aaf/android_webview/java/src/org/chromium/android_webview/AwContents.java

Requesting a merge for ca3b1cc230867486edfb16db2718ebdb63650aaf (in previous comment). Our original workaround fixed the situation for most users: this CL extends the same workaround for Samsung users with the other mail app. Requesting M-66 based on the priority indicated by Samsung in http://b/70696775#comment59.

This is low-risk, as it's guarded by package name (no effect on other apps)

This is in the most recent canary, so I'll wait until Monday before merging. I've manually verified the canary (67.0.3390.0) on my Galaxy S6.

This bug requires manual review: We are only 10 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Merge approved upon verification of the fix in canary.

I manually verified in comment #80, and this hasn't caused any issues since it rolled out Friday, so I'll proceed with merging. Thanks!

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

commit 94e1940ca6bf6ac3aaf0f9993b6265c489950c44
Author: Nate Fischer <ntfschr@chromium.org>
Date: Mon Apr 09 22:50:09 2018

AW: extend Samsung workaround for other mail app

This relaxes the check for the Samsung workaround landed in
http://crrev/c/924115, so that the workaround applies for Samsung's
other mail app which also reproduces this behavior.

Although the other mail app is referred to as
"com.samsung.android.email.provider" in our communication with them, its
implementation is actually split among 5 different APKs, and it is
"*.composer" which actually exhibits the bug.

As it turns out, the bug reproduces under the exact same sequence of
URL navigations as the other app (a file:// URL followed by
loadDataWithBaseURL("email://")).

As with http://crrev/c/924115, this is also meant as a temporary
workaround.

Bug: 781535
Test: Use both Samsung apps, test the 3 repro conditions listed in b/70696775.
Change-Id: Ic897a33dc52b78e83eaada5b4b66c555a0eb3679
Reviewed-on: https://chromium-review.googlesource.com/981691
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#548280}(cherry picked from commit ca3b1cc230867486edfb16db2718ebdb63650aaf)
Reviewed-on: https://chromium-review.googlesource.com/1002753
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#633}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/94e1940ca6bf6ac3aaf0f9993b6265c489950c44/android_webview/java/src/org/chromium/android_webview/AwContents.java

Lowering priority because workarounds have landed