Updated CRWSessionController interface to use NavigationItems.

ios/web/public/navigation_item_list.h

+// Copyright 2017 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.
+
+#ifndef IOS_WEB_PUBLIC_NAVIGATION_ITEM_LIST_H_
+#define IOS_WEB_PUBLIC_NAVIGATION_ITEM_LIST_H_
+
+#include <memory>
+#include <vector>
+
+namespace web {
+
+class NavigationItem;
+
+// Convenience typedef for a list of raw NavigationItem pointers.
+typedef std::vector<NavigationItem*> NavigationItemList;

[Eugene But (OOO till 7-30), 2017/02/03 23:33:33]

Is there a value in having this typedef? It does not make code much shorter or
clearer. To me it is misleading, because NavigationItemList is not a list and
when I see types like this I have to actually look into the header to see the
actual type.

Typedefs like this make code less clear and |auto| is a good way to address long
types for variables where necessary.

[kkhorimoto, 2017/02/04 02:45:31]

This is mostly for convenience because the actual type is unwieldy to write all
the time (especially std::vector<std::unique_ptr<web::NavigationItemImpl>>). 
It's long enough that it makes formatting a nightmare.  I named it
NavigationItemList since it's a list (not an std::list, but a list in terms of
the software engineering concept
https://en.wikipedia.org/wiki/List_(abstract_data_type) ).  I suppose I can see
how it can be a little ambiguous since it isn't obvious from the name that it's
a list of NavigationItem pointers instead of the objects themselves, but I feel
that the slight ambiguity is outweighed by the code readability.  Moreover,
using auto (which would be necessary to have reasonable formatting in many
places) would introduce far more ambiguity than the one-time cost of learning
what the typedef means.

[kkhorimoto, 2017/02/04 02:45:31]

Additionally, I modeled this typedef after RedirectList, which is a
std::vector<GURL> used all over the place in the history component.  I don't
think it's unreasonable to expect people writing code in a particular module, be
it ios/web/ or components/history, to familiarize themselves with associated
typedefs.

[Eugene But (OOO till 7-30), 2017/02/04 03:34:51]

Shouldn't we optimize code for reading and not for writing? I believe this
typedef actually hurts readability. It is quite clear what
|std::vector<NavigationItem*| means while I had to look up what
NavigationItemList is.

std::vector<std::unique_ptr<web::NavigationItemImpl>> is really bad, but is it
the case that formatting can not be improved by using auto?
I could not find a place where ScopedNavigationItemImplList really improves
formatting, except CRWSessionController. But at this point should we really care
how CRWSessionController looks? It will be removed anyways.

+
+// Convenience typedef for a list of scoped NavigationItem pointers.
+typedef std::vector<std::unique_ptr<NavigationItem>> ScopedNavigationItemList;

[Eugene But (OOO till 7-30), 2017/02/03 23:33:33]

ditto

+
+// Returns a NavigationItemList populated with raw pointer values from
+// |scoped_list|.
+NavigationItemList CreateUnscopedNavigationItemList(
+    const ScopedNavigationItemList& scoped_list);
+
+}  // namespace web
+
+#endif  // IOS_WEB_PUBLIC_NAVIGATION_ITEM_LIST_H_