[ios] Fix omnibox baseWritingDirection and dismissal iOS11 bugs.

ios/chrome/browser/ui/omnibox/omnibox_popup_material_view_controller.mm

 // Copyright (c) 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ios/chrome/browser/ui/omnibox/omnibox_popup_material_view_controller.h"
 
 #include <memory>
 
 #include "base/ios/ios_util.h"
 #include "base/mac/scoped_cftyperef.h"
@@ skipping 545 matching lines @@
 
   size_t size = _currentResult.size();
   if (animation && size > 0) {
     [self fadeInRows];
   }
 }
 
 - (void)layoutRows {
   size_t size = _currentResult.size();
 
   [self.tableView reloadData];

[rohitrao (ping after 24h), 2017/06/14 14:54:11]

I don't understand this code at all.  Why do we need to update rows after
calling reloadData?

[justincohen, 2017/06/14 16:44:08]

material design!  we lay out the rows ourselves below.  

-  [self.tableView beginUpdates];
-  for (size_t i = 0; i < kRowCount; i++) {
-    OmniboxPopupMaterialRow* row = _rows[i];
 
-    if (i < size) {
-      const AutocompleteMatch& match =
-          ((const AutocompleteResult&)_currentResult).match_at((NSUInteger)i);
-      [self updateRow:row withMatch:match];
-      row.hidden = NO;
-    } else {
-      row.hidden = YES;
+  [UIView performWithoutAnimation:^{

[rohitrao (ping after 24h), 2017/06/14 14:54:11]

Please add a comment explaining what's going on here and why we need
performWithoutAnimation.

[justincohen, 2017/06/14 16:44:08]

Done.

+    [self.tableView beginUpdates];
+    for (size_t i = 0; i < kRowCount; i++) {
+      OmniboxPopupMaterialRow* row = _rows[i];
+
+      if (i < size) {
+        const AutocompleteMatch& match =
+            ((const AutocompleteResult&)_currentResult).match_at((NSUInteger)i);
+        [self updateRow:row withMatch:match];
+        row.hidden = NO;
+      } else {
+        row.hidden = YES;
+      }
     }
-  }
-  [self.tableView endUpdates];
+    [self.tableView endUpdates];
+  }];
 
   if (IsIPadIdiom())
     [self updateContentInsetForKeyboard];
 }
 
 - (void)keyboardDidShow:(NSNotification*)notification {
   NSDictionary* keyboardInfo = [notification userInfo];
   NSValue* keyboardFrameValue =
       [keyboardInfo valueForKey:UIKeyboardFrameEndUserInfoKey];
   keyboardHeight_ = CurrentKeyboardHeight(keyboardFrameValue);
@@ skipping 58 matching lines @@
   // Make a defensive copy of |match.contents|, as CopyToOmnibox() will trigger
   // a new round of autocomplete and modify |_currentResult|.
   base::string16 contents(match.contents);
   _popupView->CopyToOmnibox(contents);
 }
 
 #pragma mark -
 #pragma mark UIScrollViewDelegate
 
 - (void)scrollViewDidScroll:(UIScrollView*)scrollView {
-  // Setting the top inset of the scrollView to |kTopAndBottomPadding| causes a
-  // one time scrollViewDidScroll to |-kTopAndBottomPadding|.  It's easier to
-  // just ignore this one scroll tick.
-  if (scrollView.contentOffset.y == 0 - kTopAndBottomPadding)
+  if (!scrollView.dragging)

[rohitrao (ping after 24h), 2017/06/14 14:54:11]

Nice, this feels much cleaner.

[justincohen, 2017/06/14 16:44:08]

good, not planning on putting this or above behind iOS11 flags.

     return;
 
   _popupView->DidScroll();
   for (OmniboxPopupMaterialRow* row in _rows) {
     row.highlighted = NO;
   }
 }
 
 // Set text alignment for popup cells.
 - (void)setTextAlignment:(NSTextAlignment)alignment {
@@ skipping 113 matching lines @@
     // The delete button never disappears if you don't call this after a tap.
     // It doesn't seem to be required anywhere else.
     [_rows[indexPath.row] prepareForReuse];
     const AutocompleteMatch& match =
         ((const AutocompleteResult&)_currentResult).match_at(indexPath.row);
     _popupView->DeleteMatch(match);
   }
 }
 
 @end