Allow URL loading if WebState::GetView was not called.

Allow URL loading if WebState::GetView was not called.

Calling GetView should not be a requirement for loading a URL. After
this CL all triggerPendingLoad calls in //ios/chrome should be
unnecessary.

BUG=705819, 616244

[Eugene But (OOO till 7-30), 2017-03-28 22:03:31]

The CQ bit was checked by eugenebut@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-28 22:04:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-28 22:16:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-28 22:16:43]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Eugene But (OOO till 7-30), 2017-03-28 22:57:22]

eugenebut@chromium.org changed reviewers:
+ kkhorimoto@chromium.org, sdefresne@chromium.org

[Eugene But (OOO till 7-30), 2017-03-28 22:57:23]

PTAL This should remove the need to call triggerPendingLoad in ios/chrome

[kkhorimoto, 2017-03-29 00:40:02]

lgtm

[sdefresne, 2017-03-29 09:59:01]

The tests are failing with the following error:

[9996:1027:0328/174407.240400:6712194424084:FATAL:crw_web_controller.mm(4024)]
Check failed: _webUsageEnabled. Tried to create a web view while suspended!

TabModel does start the load even if web usage is disabled. I guess it expect
that the load will happens later when the web usage is enabled. I don't think it
should be the responsibility of Tab/TabModel to remember the load params for
later use when web is enabled.

https://codereview.chromium.org/2777243004/diff/1/ios/web/web_state/ui/crw_we...
File ios/web/web_state/ui/crw_web_controller.mm (right):

https://codereview.chromium.org/2777243004/diff/1/ios/web/web_state/ui/crw_we...
ios/web/web_state/ui/crw_web_controller.mm:1845: // never displayed. Bail, and
let the URL be loaded when the tab is shown.
I think this comment is now incorrect (since the code does not "bail"). Please
update.

[sdefresne, 2017-03-29 10:22:21]

On 2017/03/29 09:59:01, sdefresne wrote:
> The tests are failing with the following error:
> 
> [9996:1027:0328/174407.240400:6712194424084:FATAL:crw_web_controller.mm(4024)]
> Check failed: _webUsageEnabled. Tried to create a web view while suspended!
> 
> TabModel does start the load even if web usage is disabled. I guess it expect
> that the load will happens later when the web usage is enabled. I don't think
it
> should be the responsibility of Tab/TabModel to remember the load params for
> later use when web is enabled.
> 
>
https://codereview.chromium.org/2777243004/diff/1/ios/web/web_state/ui/crw_we...
> File ios/web/web_state/ui/crw_web_controller.mm (right):
> 
>
https://codereview.chromium.org/2777243004/diff/1/ios/web/web_state/ui/crw_we...
> ios/web/web_state/ui/crw_web_controller.mm:1845: // never displayed. Bail, and
> let the URL be loaded when the tab is shown.
> I think this comment is now incorrect (since the code does not "bail"). Please
> update.

Looks like this will potentially also break ContextualSearchResultsView. If you
look at ContextualSearchResultsView -createTabForSearch:preloadEnabled:, there
is a comment saying "Don't actually start the page load yet -- that happens in
-loadTab". And the -loadTab method does create the view by calling
WebState::GetView() to force the load to start. Again, not sure how to fix this
(maybe the ContextualSearchResultsView can set web usage enabled to false and
then later set it to true when it wants the page to load).

[sdefresne, 2017-03-29 10:23:43]

On 2017/03/29 10:22:21, sdefresne wrote:
> On 2017/03/29 09:59:01, sdefresne wrote:
> > The tests are failing with the following error:
> > 
> >
[9996:1027:0328/174407.240400:6712194424084:FATAL:crw_web_controller.mm(4024)]
> > Check failed: _webUsageEnabled. Tried to create a web view while suspended!
> > 
> > TabModel does start the load even if web usage is disabled. I guess it
expect
> > that the load will happens later when the web usage is enabled. I don't
think
> it
> > should be the responsibility of Tab/TabModel to remember the load params for
> > later use when web is enabled.
> > 
> >
>
https://codereview.chromium.org/2777243004/diff/1/ios/web/web_state/ui/crw_we...
> > File ios/web/web_state/ui/crw_web_controller.mm (right):
> > 
> >
>
https://codereview.chromium.org/2777243004/diff/1/ios/web/web_state/ui/crw_we...
> > ios/web/web_state/ui/crw_web_controller.mm:1845: // never displayed. Bail,
and
> > let the URL be loaded when the tab is shown.
> > I think this comment is now incorrect (since the code does not "bail").
Please
> > update.
> 
> Looks like this will potentially also break ContextualSearchResultsView. If
you
> look at ContextualSearchResultsView -createTabForSearch:preloadEnabled:, there
> is a comment saying "Don't actually start the page load yet -- that happens in
> -loadTab". And the -loadTab method does create the view by calling
> WebState::GetView() to force the load to start. Again, not sure how to fix
this
> (maybe the ContextualSearchResultsView can set web usage enabled to false and
> then later set it to true when it wants the page to load).

And PreloadController has a call to [tab view] with a comment "// Trigger the
page to start loading.". This should probably be removed as part of this CL.

[Eugene But (OOO till 7-30), 2017-03-30 01:26:01]

On 2017/03/29 10:23:43, sdefresne wrote:
> On 2017/03/29 10:22:21, sdefresne wrote:
> > On 2017/03/29 09:59:01, sdefresne wrote:
> > > The tests are failing with the following error:
> > > 
> > >
> [9996:1027:0328/174407.240400:6712194424084:FATAL:crw_web_controller.mm(4024)]
> > > Check failed: _webUsageEnabled. Tried to create a web view while
suspended!
> > > 
> > > TabModel does start the load even if web usage is disabled. I guess it
> expect
> > > that the load will happens later when the web usage is enabled. I don't
> think
> > it
> > > should be the responsibility of Tab/TabModel to remember the load params
for
> > > later use when web is enabled.
> > > 
> > >
> >
>
https://codereview.chromium.org/2777243004/diff/1/ios/web/web_state/ui/crw_we...
> > > File ios/web/web_state/ui/crw_web_controller.mm (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2777243004/diff/1/ios/web/web_state/ui/crw_we...
> > > ios/web/web_state/ui/crw_web_controller.mm:1845: // never displayed. Bail,
> and
> > > let the URL be loaded when the tab is shown.
> > > I think this comment is now incorrect (since the code does not "bail").
> Please
> > > update.
> > 
> > Looks like this will potentially also break ContextualSearchResultsView. If
> you
> > look at ContextualSearchResultsView -createTabForSearch:preloadEnabled:,
there
> > is a comment saying "Don't actually start the page load yet -- that happens
in
> > -loadTab". And the -loadTab method does create the view by calling
> > WebState::GetView() to force the load to start. Again, not sure how to fix
> this
> > (maybe the ContextualSearchResultsView can set web usage enabled to false
and
> > then later set it to true when it wants the page to load).
> 
> And PreloadController has a call to [tab view] with a comment "// Trigger the
> page to start loading.". This should probably be removed as part of this CL.
Sorry for sending without waiting for tests to complete. Turns out fixing this
harder than I thought as Chrome code already started making the assumptions that
LoadURL does not actually load anything. After we implemented non-modal
incognito "WebUsage" is almost never false (one case I can think of is during
cookies clearing, but in that case UI has modal spinner), so I thought that this
fix will be safe. Well, I guess for now we can workaround triggerPendingLoad
with WebView::GetView() call :(

Closing this CL as it requires to fix Contextual Search first (which is not
launched yet, and it's future is complicated).

[Eugene But (OOO till 7-30), 2017-03-30 01:26:54]

Description was changed from

==========
Allow URL loading if WebState::GetView was not called.

Calling GetView should not be a requirement for loading a URL. After
this CL all triggerPendingLoad calls in //ios/chrome should be
unnecessary.

BUG=705819,616244
==========

to

==========
Allow URL loading if WebState::GetView was not called.

Calling GetView should not be a requirement for loading a URL. After
this CL all triggerPendingLoad calls in //ios/chrome should be
unnecessary.

BUG=705819,616244
==========