New issue
Advanced search Search tips

Issue 776438 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Task

Blocked on:
issue 781916
issue 788464

Blocking:
issue 734150



Sign in to add a comment

LoadIfNecessaryTest.DisableAndReenableWebUsage fails with WKBasedNavigationManager

Project Member Reported by danyao@chromium.org, Oct 19 2017

Issue description

This test fails the expectation on line 1026:
../../ios/web/web_state/ui/crw_web_controller_unittest.mm:1026: Failure
Value of: test::WaitForWebViewContainingText(web_state_.get(), "pony")
  Actual: false
Expected: true

It looks like the WKWebView is freed when web_state_->SetWebUsageEnabled(false) in line 1020. The history needs to be restored when recreating WKWebView.

What is the purpose of SetWebUsageEnabled? Perhaps another workaround should be not freeing WKWebView.
 

Comment 1 by danyao@chromium.org, Oct 27 2017

Owner: danyao@chromium.org
Status: Started (was: Available)

Comment 2 by danyao@chromium.org, Oct 27 2017

Reading through commit history, it seems that SetWebUsageEnabled() was introduced to release the web view when it is not needed (i.e. showing a native controller). My understanding is that this is to manage memory usage.

With the switch to WKWebView and WebKit, the rendering engine is in a separate web process from the app process. When I talked with eugenebut@ and kkhorimoto@ offline, they believe the cost of keeping WKWebView alive now should not be as significant as before.

With new navigation manager, we will always need a web view anyways even for native controller URL. I propose we stop freeing the webview. Longer term, we may be able to deprecate the SetWebUsageEnabled() API.

Comment 3 by danyao@chromium.org, Nov 21 2017

Blockedon: 781916

Comment 4 by danyao@chromium.org, Nov 24 2017

Blockedon: 788452

Comment 5 by danyao@chromium.org, Nov 24 2017

Blockedon: -788452 788464

Comment 6 by danyao@chromium.org, Nov 28 2017

Status: Fixed (was: Started)

Sign in to add a comment