Deprecate NavigationToFirstContentfulPaint.UserGesture reload variant

Deprecate NavigationToFirstContentfulPaint.UserGesture reload variant

I expected user_gesture flag is true when users interact with
pull-to-refresh actions or reload button in the navigation UI.
But it's always false on mobiles and desktop.

On the other hand, I confirmed that ui::PageTransitionCoreType
could not be ui::PAGE_TRANSITION_RELOAD if the reload is initiated
by JavaScript or <meta http-equiv="refresh">.

This means NavigationToFirstContentfulPaint already filters out
all cases we concern. So, let me deprecate .UserGesture variant.

BUG=607063

[Takashi Toyoshima, 2016-06-01 06:55:15]

Description was changed from

==========
Deprecate NavigationToFirstContentfulPaint.UserGesture reload variant

Since it turned out that user gesture isn't true for usual reload
operations, and cases we concerned were excluded currently without
user gesture flag, let deprecate .UserGesture variant.

BUG=607063
==========

to

==========
Deprecate NavigationToFirstContentfulPaint.UserGesture reload variant

Since it turned out that user gesture isn't true for usual reload
operations, and cases we concerned were excluded currently without
user gesture flag, let deprecate .UserGesture variant.

BUG=607063
==========

[Takashi Toyoshima, 2016-06-01 06:55:15]

toyoshim@chromium.org changed reviewers:
+ bmcquade@chromium.org, csharrison@chromium.org, isherman@chromium.org

[Takashi Toyoshima, 2016-06-01 06:55:36]

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

[commit-bot: I haz the power, 2016-06-01 06:55:56]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2028953002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2028953002/1

[Takashi Toyoshima, 2016-06-01 07:17:50]

PTAL

[commit-bot: I haz the power, 2016-06-01 08:17:42]

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

[commit-bot: I haz the power, 2016-06-01 08:17:43]

Dry run: This issue passed the CQ dry run.

[Ilya Sherman, 2016-06-01 17:56:18]

lgtm

[Charlie Harrison, 2016-06-02 13:19:00]

Can you update the description with more context? What exactly is wrong with the
user gesture bit?

[Bryan McQuade, 2016-06-02 18:43:20]

On 2016/06/02 at 13:19:00, csharrison wrote:
> Can you update the description with more context? What exactly is wrong with
the user gesture bit?

I think the user gesture information for pull to refresh is not properly
communicated to the navigation code. Takashi, can you open a bug for that? I'm
not sure who to file it to - perhaps whoever owns NavigationController. I can't
find an existing bug for this.

Here's code on the Java side that handles the pull to refresh gesture:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/android/jav...

This calls through to NavigationControllerImpl::ReloadToRefreshContent on the
C++ side, which drives a reload navigation which eventually creates a
NavigationRequest via NavigationRequest::CreateBrowserInitiated:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fr...

CreateBrowserInitiated initializes has_user_gesture to false:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fr...

It appears that has_user_gesture can only be set to true for renderer-initiated
navigations in the current implementation, in
NavigationRequest::CreateRendererInitiated.

[Takashi Toyoshima, 2016-06-07 10:17:35]

Description was changed from

==========
Deprecate NavigationToFirstContentfulPaint.UserGesture reload variant

Since it turned out that user gesture isn't true for usual reload
operations, and cases we concerned were excluded currently without
user gesture flag, let deprecate .UserGesture variant.

BUG=607063
==========

to

==========
Deprecate NavigationToFirstContentfulPaint.UserGesture reload variant

I expected user_gesture flag is true when users interact with
pull-to-refresh actions or reload button in the navigation UI.
But it's always false on mobiles and desktop.

On the other hand, I confirmed that ui::PageTransitionCoreType
could not be ui::PAGE_TRANSITION_RELOAD if the reload is initiated
by JavaScript or <meta http-equiv="refresh">.

This means NavigationToFirstContentfulPaint already filters out
all cases we concern. So, let me deprecate .UserGesture variant.

BUG=607063
==========

[Takashi Toyoshima, 2016-06-07 10:22:45]

csharrison: I updated the CL description to explain more clearly. How about
this?

Bryan:
I filed a bug at crbug.com/617904 to discuss what's the right behavior for user
initiated navigation happening in the browser process.

[Takashi Toyoshima, 2016-06-09 05:47:04]

It seems csharrison is planning to set the gesture flag for loading triggered
through navigation UIs.
So, now I feel this is worth monitoring for a while so that we can confirm there
are no other unexpected auto reload cases that makes difference between
NavigationToFirstContentfulPaint and
NavigationToFirstContentfulPaint.UserGesture.

What do you think? If you agreed, I'll just close this CL.