Hi kkhorimoto, creis, Did you have time to take a look at this? 

Please note that this is blocking issue 671963 (an M57 RBS for Bling) so getting resolution/agreement on the path forward to provide a smooth user experience would be great. 

Thank you.

>> IIUC, |CRWWebController loadCurrentURL| should load the URL and not the virtualURL. Is that correct?
This sounds correct to me.

Olivier, what exactly would you like to see to consider this bug as fixed?

The documentation states that VirtualURL is a visual thing, but the URL is sent to the "renderer". In our case, the VirtualURL is sent to the WebController.
This seems an error to me.

So we should fix this to send URL to the WebController and the content equivalent.

[self currentNavigationURL] is called 16 times in CRWWebController. In some cases taking virtual URL is correct in some cases it's not. I don't know if we can properly refactor CRWWebController in it's current shape before M57.

Oliver, do you see any alternative ways to fix reading list problem quickly? I agree that this bug must be fixed, but I'm worrying about the timing.

I think one alternative is fixing crbug.com/671160
which means check each URL before loading and if a distilled version exists, fallback to distill version if there is no network (the same behavior as Android).

This was not targetted for V1, but I don't think it is technically complicated.
This would solve the issue as loading the VirtualURL would fallback to offline version if needed.

WDYT?

Thanks Olivier. Where crbug.com/671160 fix would live? If in chrome then it sounds like a perfect short term solution. If you have to make the changes in ios/web, then it depends on the fix (ios/web should no know anything about reading list logic).

I confess that I did not have time to think a lot on that yet.

I think it is a lot similar to the web_content/native_content dispatch, so may be BVC is the good place?
Or add a permanent WebStateObserver that check the URL being loaded, and if a distilled version ezists, starts a timer?

Thanks! Solving this with WebStateObserver sounds like a perfect approach to me.

OK, I will try that tomorrow. Thanks.

I agree with Eugene that it'd be better to use the WSO technique as a workaround for M57, as auditing the various calls to |-currentNavigationURL| in CRWWebController may take a longer time and is more error prone.

Sorry for the delayed reply.  Yes, I think virtual URL is intended to be display only, with the URL being what actually gets loaded.  FWIW, I've never been able to fully understand the rewriting logic in BrowserURLHandler, though I'm not sure if that matters here.

The BrowserURLRewriter (the iOS version of BrowserURLHandler) can be used to rewrite the URL used for a particular page load.  As an example, when performing a Voice Search query, we add some extra parameters to the URL to specify the language that should be used for the text-to-speech data that is returned.  However, this only occurs when creating a new navigation, and does not update the virtual URL.

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

commit f96b83410bafa951fcddfef3d225964937bac581
Author: olivierrobin <olivierrobin@chromium.org>
Date: Mon Dec 19 09:59:41 2016

Remove CRWWebController currentNavigationURL

This method returns the VirtualURL of the current navigation and is used at some
places where GetURL should be used instead.
Remove the method to avoid confusion.
Further CLs will audit which usage should be VirtualURL and which should be URL.

BUG= 671964 

Review-Url: https://codereview.chromium.org/2581193002
Cr-Commit-Position: refs/heads/master@{#439445}

[modify] https://crrev.com/f96b83410bafa951fcddfef3d225964937bac581/ios/web/web_state/ui/crw_web_controller.mm

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

commit e948fa4c8f393ae2b69ba38183632e698cde8cfe
Author: olivierrobin <olivierrobin@chromium.org>
Date: Tue Dec 20 09:09:53 2016

Call registerLoadRequest with URL instead of VirtualURL.

registerLoadRequest must take a URL as it is comparing it with a URL.

BUG= 671964 

Review-Url: https://codereview.chromium.org/2582373002
Cr-Commit-Position: refs/heads/master@{#439756}

[modify] https://crrev.com/e948fa4c8f393ae2b69ba38183632e698cde8cfe/ios/web/web_state/ui/crw_web_controller.mm

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

commit 7cba97f95ce2d1569a68bd7b20ef7e06b15b8924
Author: olivierrobin <olivierrobin@chromium.org>
Date: Tue Dec 20 18:04:14 2016

Pass URL to nativeProviders.

Pass the URL instead of the VirtualURL to the native providers and the
webUI.

BUG= 671964 

Review-Url: https://codereview.chromium.org/2590803002
Cr-Commit-Position: refs/heads/master@{#439842}

[modify] https://crrev.com/7cba97f95ce2d1569a68bd7b20ef7e06b15b8924/ios/web/web_state/ui/crw_web_controller.mm

VirtualURL vs. URL has been disambiguated now.  VirtualURL is used to show in the omnibox/create display titles, while URL is what's sent to WKWebView