Removed -[CRWWebDelegate webWillInitiateLoadWithParams:].

ios/web/web_state/ui/crw_web_controller.mm

Index: ios/web/web_state/ui/crw_web_controller.mm
diff --git a/ios/web/web_state/ui/crw_web_controller.mm b/ios/web/web_state/ui/crw_web_controller.mm
index e99824fd4b2e0078199b44b1d96eb491b1e00521..6e5ae32c21d697a687a03e16476677f7c5f03dcd 100644
--- a/ios/web/web_state/ui/crw_web_controller.mm
+++ b/ios/web/web_state/ui/crw_web_controller.mm
@@ -1791,20 +1791,9 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
   [self loadNativeViewWithSuccess:YES];
 }
 
-- (void)loadWithParams:(const NavigationManager::WebLoadParams&)originalParams {
-  // Make a copy of |params|, as some of the delegate methods may modify it.
-  NavigationManager::WebLoadParams params(originalParams);
+- (void)loadWithParams:(const NavigationManager::WebLoadParams&)params {
+  DCHECK(!(params.transition_type & ui::PAGE_TRANSITION_FORWARD_BACK));
 
-  // Initiating a navigation from the UI, record the current page state before
-  // the new page loads.
-
-  [_delegate webWillInitiateLoadWithParams:params];

[sdefresne, 2017/04/03 13:46:01]

It looks like this is introducing a functional change.
-webWillInitiateLoadWithParams: says that it want to count a crash occurring
after loading started as a non-startup crash. However, with this change the
method is only called after the load actually started. This mean that if there
is a crash between line 1801-1837 (of the old file), then it will now be counted
as a startup crash while it was not counted like that before.

+rohirao: is this functional change okay?

[rohitrao (ping after 24h), 2017/04/04 01:49:34]

Is there any way to avoid making a functional change here?  Is there a
WebStateDelegate or WebStateObserver method that gets called anywhere near here,
or is webDidUpdateSessionForLoadWithParams: the very first hook point that we
have?  Who calls loadWithParams?  Does the caller have an observer method that
we can use?

If there's no other way to hook in, then this is probably ok, but there's a lot
of work between lines 1801-1837.  Worst case we'll end up incorrectly flagging
crashes as a startup crash loop, which is what the original code was trying to
avoid.

[Eugene But (OOO till 7-30), 2017/04/04 02:08:47]

Currently there is no hook and the plan was to have only one DidStartNavigation
callback to avoid unnecessary complexity.

loadWithParams is called for all kinds of URL loads, but for the purpose of this
CL it it fair to say that |loadWithParams| will be called once foreground tab is
displayed to the user.

I would argue that if code crashes between lines 1801-1837 it is no different
than a startup crash (at least from the user perspective). WDYT?

Updating CL description to reflect this discussion.

-
-  GURL navUrl = params.url;
-  ui::PageTransition transition = params.transition_type;
-  DCHECK(!(transition & ui::PAGE_TRANSITION_FORWARD_BACK));
-
-  BOOL initialNavigation = NO;
   // Clear transient view before making any changes to history and navigation
   // manager. TODO(stuartmorgan): Drive Transient Item clearing from
   // navigation system, rather than from WebController.
@@ -1812,16 +1801,15 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
 
   [self recordStateInHistory];
 
-  if (!self.currentNavItem)
-    initialNavigation = YES;
+  BOOL initialNavigation = !self.currentNavItem;
 
   web::NavigationInitiationType navigationInitiationType =
       params.is_renderer_initiated
           ? web::NavigationInitiationType::RENDERER_INITIATED
           : web::NavigationInitiationType::USER_INITIATED;
   self.navigationManagerImpl->AddPendingItem(
-      navUrl, params.referrer, transition, navigationInitiationType,
-      params.user_agent_override_option);
+      params.url, params.referrer, params.transition_type,
+      navigationInitiationType, params.user_agent_override_option);
 
   web::NavigationItemImpl* addedItem = self.currentNavItem;
   DCHECK(addedItem);