It turns out that we may still need a user agent override flag because of pre-load. I thought we can add |UserAgentType| to WebLoadParams, and update |AddPendingItem| to take an |UserAgentType| parameter, but it's not sufficient because in some cases, we want the newly added pending to inherit user agent type from last committed item, and there is no way to represent this option using UserAgentType alone.

Content has the following enum in NavigationController:

  enum UserAgentOverrideOption {
    // Use the override value from the previous NavigationEntry in the
    // NavigationController.
    UA_OVERRIDE_INHERIT,

    // Use the default user agent.
    UA_OVERRIDE_FALSE,

    // Use the user agent override, if it's available.
    UA_OVERRIDE_TRUE

    // Adding new UserAgentOverrideOption? Also update LoadUrlParams.java
    // static constants.
  };

So, I think the best solution would be to implement the same thing in NavigationManager:

  enum class UserAgentOverrideOption : short {
    // Use the override value from the last committed non-app specific item in the
    // NavigationManager.
    UA_OVERRIDE_INHERIT,

    // Use the mobile user agent.
    UA_OVERRIDE_MOBILE,

    // Use the desktop user agent.
    UA_OVERRIDE_DESKTOP
  };

What do you think?

>>> What do you think?
Seems reasonable.

That sounds good to me.

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

commit 60323989ade77aa658a4e5b4974b6673b14fc399
Author: liaoyuke <liaoyuke@chromium.org>
Date: Thu Mar 30 23:13:38 2017

implement user agent override option.

This CL implements the UserAgentOverrideOption enum class in
NavigationManager to replace the
override_desktop_user_agent_for_next_pending_item_ flag, and also added
corresponding unit tests to verify that UserAgentOverrideOption is
working as intended.

BUG=692303

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

[modify] https://crrev.com/60323989ade77aa658a4e5b4974b6673b14fc399/ios/chrome/browser/tabs/tab.h
[modify] https://crrev.com/60323989ade77aa658a4e5b4974b6673b14fc399/ios/chrome/browser/tabs/tab.mm
[modify] https://crrev.com/60323989ade77aa658a4e5b4974b6673b14fc399/ios/chrome/browser/tabs/tab_unittest.mm
[modify] https://crrev.com/60323989ade77aa658a4e5b4974b6673b14fc399/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/60323989ade77aa658a4e5b4974b6673b14fc399/ios/web/navigation/navigation_manager_impl.h
[modify] https://crrev.com/60323989ade77aa658a4e5b4974b6673b14fc399/ios/web/navigation/navigation_manager_impl.mm
[modify] https://crrev.com/60323989ade77aa658a4e5b4974b6673b14fc399/ios/web/navigation/navigation_manager_impl_unittest.mm
[modify] https://crrev.com/60323989ade77aa658a4e5b4974b6673b14fc399/ios/web/public/navigation_manager.h
[modify] https://crrev.com/60323989ade77aa658a4e5b4974b6673b14fc399/ios/web/public/test/fakes/test_navigation_manager.h
[modify] https://crrev.com/60323989ade77aa658a4e5b4974b6673b14fc399/ios/web/public/test/fakes/test_navigation_manager.mm
[modify] https://crrev.com/60323989ade77aa658a4e5b4974b6673b14fc399/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/60323989ade77aa658a4e5b4974b6673b14fc399/ios/web/web_state/ui/crw_web_controller_unittest.mm

It turns out fixing this bug is non-trivial due to some existing bugs and technical debts :)

For example, current WKWebViewNavigationBlock does not allow reloading if current item's URL is different from WKWebView's URL etc, which is a problem for us because we change current item's URL to its original request URL. etc.

Given that Android is going to implement a smarter user agent mechanism, I think we should just keep the current behavior (adding a new pending item for Request Desktop/Mobile Site). It's frustrating, but it's the right thing to do as we should not create more workarounds to make our future development harder.

We should not close the bug, just because it is hard to fix. This is still bug for various reasons: inconsistency with Android, worse UX.

Eugene -- do you think this is still the desired behavior? With #slim-navigation-manager, it actually takes more work to create a new entry, and that code is error-prone. It will be great if deleting that code fixes this bug! :)

Ideally we should not create a new entry. That's better from UX and platform consistency perspective.