New issue
Advanced search Search tips

Issue 705819 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Task



Sign in to add a comment

NavigationManager::LoadURLWithParams does not load the URL if view is not yet created

Project Member Reported by eugene...@chromium.org, Mar 28 2017

Issue description

Current workaround (done in many tests) is to call WebState::GetView().
Calling GetView should not be a requirement for loading a URL
 
Labels: -Type-Bug -Pri-2 Pri-3 Type-Feature
Owner: ----
Status: Available (was: Assigned)
Unsuccessful attempt to fix was here:
https://codereview.chromium.org/2777243004/
Labels: -Type-Feature Type-Task
Owner: danyao@chromium.org
Labels: Proj-Not-WKBackForwardList
Cc: danyao@chromium.org
Owner: ----
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 17 2017

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

commit ce6ac4c1d9101c2edeecc21c9360721d35019105
Author: Eugene But <eugenebut@google.com>
Date: Tue Oct 17 14:59:59 2017

Use LoadIfNecessary instead of GetView to trigger the load.

LoadURLWithParams should load the URL without any additional calls, but
the load does not happen because of crbug.com/705819.

WebState::GetView() will be changed so it does not trigger pending,
load hence LoadIfNecessary is used intead.

Bug: 705819
Change-Id: I1356e37c6d3d7a0d9e0739e4605bf76928d455a8
Reviewed-on: https://chromium-review.googlesource.com/721078
Reviewed-by: Eric Noyau <noyau@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509387}
[modify] https://crrev.com/ce6ac4c1d9101c2edeecc21c9360721d35019105/components/dom_distiller/ios/distiller_page_ios.mm

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 17 2017

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

commit 6ab54071f662775974467f4262858a9f22e23afb
Author: Eugene But <eugenebut@google.com>
Date: Tue Oct 17 15:01:33 2017

Do not call WebState::View() in PurgeCachedWebViewPages.

New NavigationManager::LoadIfNecessary triggers the load, which replaces
WebState::View() hacky call.

Bug: 705819
Change-Id: Ie8c13bd9eb07a982f88372ee17d2c2dd5f6a6018
Reviewed-on: https://chromium-review.googlesource.com/722223
Reviewed-by: Mike Baxley <baxley@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509389}
[modify] https://crrev.com/6ab54071f662775974467f4262858a9f22e23afb/ios/chrome/browser/web/BUILD.gn
[modify] https://crrev.com/6ab54071f662775974467f4262858a9f22e23afb/ios/chrome/browser/web/visible_url_egtest.mm

Owner: eugene...@chromium.org
Status: Assigned (was: Available)
Cc: sdefresne@chromium.org
Another unsuccessful attempt to fix was here:
https://chromium-review.googlesource.com/c/chromium/src/+/730867

This time because -[Tab view] reports DidStartPageLoad as a side effect, which breaks Tab Usage recording with cl/730867

Will try to fix this again, when -[Tab view] is killed. 
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 2 2017

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

commit 0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a
Author: Eugene But <eugenebut@google.com>
Date: Thu Nov 02 19:21:52 2017

Made WebUsage enabled by default.

When Chrome used UIWebView it was safer to have NO as default value for
webUsageEnabled property. This is because Incognito feature was modal
and non-active browser state had disabled web usage for all web
controllers.

After switching to WKWebView the only case when webUsageEnabled is NO
is during browsing data clearing. So it makes sense to use YES as the
default value for webUsageEnabled property.

Bug: 705819
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I68d22d92a2a36b07b2f89e329363681addc2f414
Reviewed-on: https://chromium-review.googlesource.com/750049
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513580}
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/components/dom_distiller/ios/distiller_page_ios.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/chrome/browser/prerender/preload_controller.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/chrome/browser/sessions/session_util.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/chrome/browser/ui/alert_coordinator/repost_form_coordinator_unittest.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/chrome/browser/ui/browser_list/browser_list_session_service_factory.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/chrome/browser/web_state_list/web_state_list_serialization_unittest.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/clean/chrome/browser/ui/omnibox/location_bar_mediator.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/web/public/test/fakes/test_web_state.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/web/public/test/fakes/test_web_state_delegate.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/web/public/test/web_test_with_web_state.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/web/shell/view_controller.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/web/test/web_int_test.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/web/web_state/ui/crw_web_controller_unittest.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/web/web_state/web_state_impl_unittest.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/web/webui/web_ui_mojo_inttest.mm
[modify] https://crrev.com/0a8cfe467cdcfa5ad3d1d5a8d1eae803650f9e9a/ios/web_view/internal/cwv_web_view.mm

Owner: ----
Status: Available (was: Assigned)
Project Member

Comment 12 by bugdroid1@chromium.org, Mar 1 2018

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

commit 83c32cb9581419ed779dd55412004dd718b7e853
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Thu Mar 01 11:03:35 2018

Use LoadIfNecessary instead of Tab -view

In PreloadController, use NavigationManager::LoadIfNecessary
instead of calling Tab -view to force the navigation.

Bug: 705819
Change-Id: If7814ebd60e1100f56bcb15d817f1d8ec04be1b2
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/941330
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540124}
[modify] https://crrev.com/83c32cb9581419ed779dd55412004dd718b7e853/ios/chrome/browser/prerender/preload_controller.mm

Sign in to add a comment