[iOS] Refactoring web CRWSessionController user agent code.

ios/web/navigation/crw_session_controller.mm

Index: ios/web/navigation/crw_session_controller.mm
diff --git a/ios/web/navigation/crw_session_controller.mm b/ios/web/navigation/crw_session_controller.mm
index aa98921e620a5bc72c68361aaeca35d56bf95462..3e4f2045fdeacf4565139e042dc1989c6fb8e54a 100644
--- a/ios/web/navigation/crw_session_controller.mm
+++ b/ios/web/navigation/crw_session_controller.mm
@@ -367,10 +367,22 @@ - (void)addPendingItem:(const GURL&)url
   CRWSessionEntry* currentEntry = self.currentEntry;
   if (currentEntry) {
     web::NavigationItem* item = [currentEntry navigationItem];
-    if (item->GetURL() == url &&
-        (!PageTransitionCoreTypeIs(trans, ui::PAGE_TRANSITION_FORM_SUBMIT) ||
-         PageTransitionCoreTypeIs(item->GetTransitionType(),
-                                  ui::PAGE_TRANSITION_FORM_SUBMIT))) {
+    BOOL isPendingItemEqualsCurrentItem = NO;

[kkhorimoto, 2017/02/15 23:02:05]

As long as we're refactoring this for readability, why don't we just define this
variable later and initialize it directly to its final value?  Additionally, it
might improve readability by naming the variable in terms of its effect in the
overall function (i.e. whether or not to create the new pending item)

BOOL shouldCreatePendingItem = !hasSameURL || isPendingTransitionFormSubmit &&
!isCurrentTransitionFormSubmit;

[liaoyuke, 2017/02/16 01:49:04]

Done.

[liaoyuke, 2017/02/16 01:49:04]

Makes sense!

+
+    BOOL hasSameUrl = item->GetURL() == url;

[kkhorimoto, 2017/02/15 23:02:06]

s/hasSameUrl/hasSameURL

[liaoyuke, 2017/02/16 01:49:04]

Done.

+    BOOL isPendingItemPageTransitionFormSubmit =
+        PageTransitionCoreTypeIs(trans, ui::PAGE_TRANSITION_FORM_SUBMIT);
+    BOOL isCurrentItemPageTransitionFormSubmit = PageTransitionCoreTypeIs(

[kkhorimoto, 2017/02/15 23:02:05]

You can make these variable names a little less verbose:

isPendingTransitionFormSubmit
isCurrentTransitionFormSubmit

[liaoyuke, 2017/02/16 01:49:05]

Done.

+        item->GetTransitionType(), ui::PAGE_TRANSITION_FORM_SUBMIT);
+

[liaoyuke, 2017/02/15 22:26:51]

No structural changes. Just happened to find this piece of code extremely hard
to read, so did a little bit refactoring.

[liaoyuke, 2017/02/16 01:49:04]

Done.

+    if (hasSameUrl) {
+      if (!isPendingItemPageTransitionFormSubmit ||
+          isCurrentItemPageTransitionFormSubmit) {
+        isPendingItemPageTransitionFormSubmit = YES;
+      }
+    }
+
+    if (isPendingItemEqualsCurrentItem) {
       // Send the notification anyway, to preserve old behavior. It's unknown
       // whether anything currently relies on this, but since both this whole
       // hack and the content facade will both be going away, it's not worth
@@ -382,15 +394,10 @@ - (void)addPendingItem:(const GURL&)url
     }
   }
 
-  BOOL useDesktopUserAgent =
-      _useDesktopUserAgentForNextPendingItem ||
-      (self.currentEntry.navigationItem &&
-       self.currentEntry.navigationItem->IsOverridingUserAgent());
-  _useDesktopUserAgentForNextPendingItem = NO;
   _pendingEntry.reset([self sessionEntryWithURL:url
                                        referrer:ref
                                      transition:trans
-                            useDesktopUserAgent:useDesktopUserAgent
+                            useDesktopUserAgent:NO

[kkhorimoto, 2017/02/15 23:02:05]

useDesktopUserAgent is always NO after this change right?  We can just remove
that parameter from the session entry creation function since it's set only in
NavigationManagerImpl.

[liaoyuke, 2017/02/16 01:49:04]

Done.

                               rendererInitiated:rendererInitiated]);
 
   if (_navigationManager && _navigationManager->GetFacadeDelegate()) {
@@ -679,13 +686,6 @@ - (CRWSessionEntry*)lastUserEntry {
   return [_entries objectAtIndex:index];
 }
 
-- (void)useDesktopUserAgentForNextPendingItem {
-  if (_pendingEntry)
-    [_pendingEntry navigationItem]->SetIsOverridingUserAgent(true);
-  else
-    _useDesktopUserAgentForNextPendingItem = YES;
-}
-
 - (NSInteger)indexOfItem:(const web::NavigationItem*)item {
   web::NavigationItemList items = self.items;
   for (NSInteger i = 0; i < static_cast<NSInteger>(items.size()); ++i) {