Showing previews UI for Offline Previews

Showing previews UI for Offline Previews

This changes the functionality of is_offline_preview() in OfflinePageTabHelper to check the provisional information as well.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features.

BUG=615564, 649148
Committed: https://crrev.com/21364834a65e541369e8895d93695dfaf114056b
Committed: https://crrev.com/e28ec590f2f8f5103fb724c587d3abc1a6782cb8
Cr-Original-Commit-Position: refs/heads/master@{#427902}
Cr-Commit-Position: refs/heads/master@{#428124}

[RyanSturm, 2016-09-22 23:00:10]

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

[commit-bot: I haz the power, 2016-09-22 23:00:31]

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

[RyanSturm, 2016-09-22 23:06:22]

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

[commit-bot: I haz the power, 2016-09-22 23:06:47]

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, 2016-09-22 23:16:31]

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

[commit-bot: I haz the power, 2016-09-22 23:16:32]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[RyanSturm, 2016-09-23 01:29:51]

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

[commit-bot: I haz the power, 2016-09-23 01:30:20]

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, 2016-09-23 02:07:48]

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

[commit-bot: I haz the power, 2016-09-23 02:07:49]

Dry run: This issue passed the CQ dry run.

[RyanSturm, 2016-09-23 16:57:38]

ryansturm@chromium.org changed reviewers:
+ megjablon@chromium.org

[RyanSturm, 2016-09-23 16:57:39]

megjablon: PTAL @ PreviewsInfoBar*

[Dmitry Titov, 2016-09-23 18:05:47]

dimich@chromium.org changed reviewers:
+ dimich@chromium.org

[Dmitry Titov, 2016-09-23 18:05:47]

Drive-by: Will this introduce a potentially quirky UX? I've added comments to
the bug, wonder what our UX experts think...

[RyanSturm, 2016-09-30 22:04:27]

Description was changed from

==========
Moving Offline Previews to Previews UI

This prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features. The "show original" functionality is still pending, so if the
user is still on a prohibitively slow network the offline page will be
shown again.

BUG=615564
==========

to

==========
Moving Offline page info through URLRequest and NavigationHandle

Instead of passing provisional information to the offline pages tab
helper when the navigation to an offline page is determined, the
information should be passed through URLRequest and NavigationHandle as
part of ChromeNavigationData. This keeps the information more closely
related to the navigation handle instead of the WebContents.
DidFinishNavigation remains the point at which the offline information
is stored on the tab helper.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features. The "show original" functionality is still pending, so if the
user is still on a prohibitively slow network the offline page will be
shown again.

BUG=615564
==========

[RyanSturm, 2016-09-30 22:07:56]

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

[commit-bot: I haz the power, 2016-09-30 22:08:27]

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

[RyanSturm, 2016-09-30 22:10:04]

ryansturm@chromium.org changed reviewers:
+ jianli@chromium.org

[RyanSturm, 2016-09-30 22:10:05]

jianli, dimich:
PTAL

[commit-bot: I haz the power, 2016-09-30 22:11:59]

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

[commit-bot: I haz the power, 2016-09-30 22:12:00]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[RyanSturm, 2016-09-30 22:26:29]

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

[commit-bot: I haz the power, 2016-09-30 22:27:03]

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, 2016-09-30 22:31:28]

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

[commit-bot: I haz the power, 2016-09-30 22:31:29]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[RyanSturm, 2016-09-30 23:13:58]

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

[commit-bot: I haz the power, 2016-09-30 23:14:35]

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, 2016-10-01 00:25:05]

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

[commit-bot: I haz the power, 2016-10-01 00:25:06]

Dry run: This issue passed the CQ dry run.

[megjablon, 2016-10-07 22:05:41]

https://codereview.chromium.org/2362033002/diff/120001/chrome/browser/preview...
File chrome/browser/previews/previews_infobar_tab_helper.cc (left):

https://codereview.chromium.org/2362033002/diff/120001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper.cc:33:
set_displayed_preview_infobar(false);
This still needs to be reset for previews, so that on main frame navigation from
one preview page to another the state is reset unless I'm mistaken. 

Shouldn't we also reset is_showing_offline_preview_ here too?

https://codereview.chromium.org/2362033002/diff/120001/chrome/browser/preview...
File chrome/browser/previews/previews_infobar_tab_helper.cc (right):

https://codereview.chromium.org/2362033002/diff/120001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper.cc:32: // Do nothing if this
is full main frame navigation.
Not sure what this comment means. We show the UI if we are navigating in the
main frame.

[RyanSturm, 2016-10-07 22:25:30]

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

[commit-bot: I haz the power, 2016-10-07 22:26:09]

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

[RyanSturm, 2016-10-07 22:26:34]

https://codereview.chromium.org/2362033002/diff/120001/chrome/browser/preview...
File chrome/browser/previews/previews_infobar_tab_helper.cc (left):

https://codereview.chromium.org/2362033002/diff/120001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper.cc:33:
set_displayed_preview_infobar(false);
On 2016/10/07 22:05:40, megjablon wrote:
> This still needs to be reset for previews, so that on main frame navigation
from
> one preview page to another the state is reset unless I'm mistaken. 
> 
> Shouldn't we also reset is_showing_offline_preview_ here too?

DidStartProvisionalLoadForFrame is deprecated in favor of DidFinishNavigation.
Is there any reason why this can't be reset in DidFinishNavigation?

https://codereview.chromium.org/2362033002/diff/120001/chrome/browser/preview...
File chrome/browser/previews/previews_infobar_tab_helper.cc (right):

https://codereview.chromium.org/2362033002/diff/120001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper.cc:32: // Do nothing if this
is full main frame navigation.
On 2016/10/07 22:05:40, megjablon wrote:
> Not sure what this comment means. We show the UI if we are navigating in the
> main frame.

Done.

[commit-bot: I haz the power, 2016-10-07 22:28:54]

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

[commit-bot: I haz the power, 2016-10-07 22:28:55]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[megjablon, 2016-10-07 22:48:33]

https://codereview.chromium.org/2362033002/diff/120001/chrome/browser/preview...
File chrome/browser/previews/previews_infobar_tab_helper.cc (left):

https://codereview.chromium.org/2362033002/diff/120001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper.cc:33:
set_displayed_preview_infobar(false);
On 2016/10/07 22:26:34, Ryan Sturm wrote:
> On 2016/10/07 22:05:40, megjablon wrote:
> > This still needs to be reset for previews, so that on main frame navigation
> from
> > one preview page to another the state is reset unless I'm mistaken. 
> > 
> > Shouldn't we also reset is_showing_offline_preview_ here too?
> 
> DidStartProvisionalLoadForFrame is deprecated in favor of DidFinishNavigation.
> Is there any reason why this can't be reset in DidFinishNavigation?

Took another look. Looks like it should work fine.

https://codereview.chromium.org/2362033002/diff/140001/chrome/browser/preview...
File chrome/browser/previews/previews_infobar_tab_helper.cc (right):

https://codereview.chromium.org/2362033002/diff/140001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper.cc:32: // Do nothing if this
is not a full main frame navigation.
Suggestion: Rather than the double negative
// Only show the infobar if this is a full main frame navigation

https://codereview.chromium.org/2362033002/diff/140001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper.cc:34:
!navigation_handle->HasCommitted() || navigation_handle->IsSamePage())
Does a refresh count as same page?

[RyanSturm, 2016-10-07 23:05:16]

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

[commit-bot: I haz the power, 2016-10-07 23:05:47]

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

[RyanSturm, 2016-10-07 23:05:48]

https://codereview.chromium.org/2362033002/diff/140001/chrome/browser/preview...
File chrome/browser/previews/previews_infobar_tab_helper.cc (right):

https://codereview.chromium.org/2362033002/diff/140001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper.cc:32: // Do nothing if this
is not a full main frame navigation.
On 2016/10/07 22:48:33, megjablon wrote:
> Suggestion: Rather than the double negative
> // Only show the infobar if this is a full main frame navigation

Done.

https://codereview.chromium.org/2362033002/diff/140001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper.cc:34:
!navigation_handle->HasCommitted() || navigation_handle->IsSamePage())
On 2016/10/07 22:48:33, megjablon wrote:
> Does a refresh count as same page? 

This checks for fragment navigations (i.e. links that scroll the page to a
specific location), not reloads.

[commit-bot: I haz the power, 2016-10-07 23:09:08]

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

[commit-bot: I haz the power, 2016-10-07 23:09:09]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[RyanSturm, 2016-10-07 23:16:38]

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

[commit-bot: I haz the power, 2016-10-07 23:16:56]

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, 2016-10-07 23:40:10]

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

[commit-bot: I haz the power, 2016-10-07 23:40:11]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[RyanSturm, 2016-10-10 16:54:54]

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

[commit-bot: I haz the power, 2016-10-10 16:55:10]

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, 2016-10-10 19:00:40]

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

[commit-bot: I haz the power, 2016-10-10 19:00:41]

Dry run: This issue passed the CQ dry run.

[megjablon, 2016-10-10 22:25:13]

https://codereview.chromium.org/2362033002/diff/140001/chrome/browser/preview...
File chrome/browser/previews/previews_infobar_tab_helper.cc (right):

https://codereview.chromium.org/2362033002/diff/140001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper.cc:34:
!navigation_handle->HasCommitted() || navigation_handle->IsSamePage())
On 2016/10/07 23:05:48, Ryan Sturm wrote:
> On 2016/10/07 22:48:33, megjablon wrote:
> > Does a refresh count as same page? 
> 
> This checks for fragment navigations (i.e. links that scroll the page to a
> specific location), not reloads.

Ok cool.

https://codereview.chromium.org/2362033002/diff/200001/chrome/browser/preview...
File chrome/browser/previews/previews_infobar_tab_helper_unittest.cc (right):

https://codereview.chromium.org/2362033002/diff/200001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper_unittest.cc:10: #include
"base/files/file_path.h"
Is this include needed? I may be missing where it's used. Same for a few below.
Can you make sure these are all needed?

https://codereview.chromium.org/2362033002/diff/200001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper_unittest.cc:59:
context_.CreateRequest(GURL("a.com"), net::IDLE, nullptr);
Can this use kTestUrl instead of "a.com"?

https://codereview.chromium.org/2362033002/diff/200001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper_unittest.cc:167:
->NavigateAndCommit(GURL(kTestUrl2));
Can you add a test that checks that when the url remains the same, the displayed
state isn't reset? Otherwise we shouldn't need a different url.

[RyanSturm, 2016-10-11 02:05:47]

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

[commit-bot: I haz the power, 2016-10-11 02:05:58]

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

[RyanSturm, 2016-10-11 02:09:56]

megjablon, jianli: PTAL

https://codereview.chromium.org/2362033002/diff/200001/chrome/browser/preview...
File chrome/browser/previews/previews_infobar_tab_helper_unittest.cc (right):

https://codereview.chromium.org/2362033002/diff/200001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper_unittest.cc:10: #include
"base/files/file_path.h"
On 2016/10/10 22:25:13, megjablon wrote:
> Is this include needed? I may be missing where it's used. Same for a few
below.
> Can you make sure these are all needed?

Done.

https://codereview.chromium.org/2362033002/diff/200001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper_unittest.cc:59:
context_.CreateRequest(GURL("a.com"), net::IDLE, nullptr);
On 2016/10/10 22:25:13, megjablon wrote:
> Can this use kTestUrl instead of "a.com"?

Done. This request is short-lived though, as it is only needed to create the
request data, which is then copied to NavigationHandle. I added comments to
clarify.

https://codereview.chromium.org/2362033002/diff/200001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_tab_helper_unittest.cc:167:
->NavigateAndCommit(GURL(kTestUrl2));
On 2016/10/10 22:25:13, megjablon wrote:
> Can you add a test that checks that when the url remains the same, the
displayed
> state isn't reset? Otherwise we shouldn't need a different url.

Done.

[commit-bot: I haz the power, 2016-10-11 02:57:58]

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

[commit-bot: I haz the power, 2016-10-11 02:58:00]

Dry run: This issue passed the CQ dry run.

[RyanSturm, 2016-10-12 18:47:47]

Description was changed from

==========
Moving Offline page info through URLRequest and NavigationHandle

Instead of passing provisional information to the offline pages tab
helper when the navigation to an offline page is determined, the
information should be passed through URLRequest and NavigationHandle as
part of ChromeNavigationData. This keeps the information more closely
related to the navigation handle instead of the WebContents.
DidFinishNavigation remains the point at which the offline information
is stored on the tab helper.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features. The "show original" functionality is still pending, so if the
user is still on a prohibitively slow network the offline page will be
shown again.

BUG=615564
==========

to

==========
Moving Offline page info through URLRequest and NavigationHandle

Instead of passing provisional information to the offline pages tab
helper when the navigation to an offline page is determined, the
information should be passed through URLRequest and NavigationHandle as
part of ChromeNavigationData. This keeps the information more closely
related to the navigation handle instead of the WebContents.
DidFinishNavigation remains the point at which the offline information
is stored on the tab helper.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features. The "show original" functionality is still pending, so if the
user is still on a prohibitively slow network the offline page will be
shown again.

BUG=615564,649148
==========

[RyanSturm, 2016-10-20 20:30:29]

ryansturm@chromium.org changed reviewers:
+ bengr@chromium.org

[RyanSturm, 2016-10-24 22:47:13]

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

[commit-bot: I haz the power, 2016-10-24 22:48:06]

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, 2016-10-24 22:57:28]

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

[commit-bot: I haz the power, 2016-10-24 22:57:29]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[RyanSturm, 2016-10-25 17:21:20]

Description was changed from

==========
Moving Offline page info through URLRequest and NavigationHandle

Instead of passing provisional information to the offline pages tab
helper when the navigation to an offline page is determined, the
information should be passed through URLRequest and NavigationHandle as
part of ChromeNavigationData. This keeps the information more closely
related to the navigation handle instead of the WebContents.
DidFinishNavigation remains the point at which the offline information
is stored on the tab helper.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features. The "show original" functionality is still pending, so if the
user is still on a prohibitively slow network the offline page will be
shown again.

BUG=615564,649148
==========

to

==========
Showing previews UI for Offline Previews

This changes the functionality of is_offline_preview() in OfflinePageTabHelper
to check the provisional information as well.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features. The "show original" functionality is still pending, so if the
user is still on a prohibitively slow network the offline page will be
shown again.

BUG=615564,649148
==========

[RyanSturm, 2016-10-25 17:21:37]

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

[commit-bot: I haz the power, 2016-10-25 17:22:16]

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

[RyanSturm, 2016-10-25 17:22:34]

jianli: I changed the CL as agreed upon; PTAL.

[commit-bot: I haz the power, 2016-10-25 18:55:12]

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

[commit-bot: I haz the power, 2016-10-25 18:55:13]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[jianli, 2016-10-25 22:14:16]

https://codereview.chromium.org/2362033002/diff/240001/chrome/android/java/sr...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://codereview.chromium.org/2362033002/diff/240001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:2883: public
boolean isShowingOfflinePreview() {
We should try to avoid adding more offline related codes into Tab. Instead, you
could put them in OfflinePageUtils (see how I did it for
getOfflinePageHeaderForReload).

FYI, this is per feedback from tedchoc in my previous review. We're going to
move offline handling logic from Tab to OfflinePageUtils.

https://codereview.chromium.org/2362033002/diff/240001/chrome/browser/android...
File chrome/browser/android/offline_pages/offline_page_tab_helper.cc (right):

https://codereview.chromium.org/2362033002/diff/240001/chrome/browser/android...
chrome/browser/android/offline_pages/offline_page_tab_helper.cc:181: }
nit: empty line

https://codereview.chromium.org/2362033002/diff/240001/chrome/browser/android...
File chrome/browser/android/offline_pages/offline_page_tab_helper.h (right):

https://codereview.chromium.org/2362033002/diff/240001/chrome/browser/android...
chrome/browser/android/offline_pages/offline_page_tab_helper.h:45: bool
IsOfflinePreview() const;
nit: rename to IsShowingOfflinePreview in order to be consistent with other
places

Also, please change is_offline_preview to is_showing_offline_preview in
LoadedOfflinePageInfo.

https://codereview.chromium.org/2362033002/diff/240001/chrome/browser/android...
File chrome/browser/android/offline_pages/offline_page_utils.h (right):

https://codereview.chromium.org/2362033002/diff/240001/chrome/browser/android...
chrome/browser/android/offline_pages/offline_page_utils.h:47: // Returns whether
the offline page corresponding to |web_contents| is part of
The comment is a bit hard to understand. How about:
  // Returns true if the offline page is shown for previewing purpose.

[RyanSturm, 2016-10-25 23:00:13]

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

[commit-bot: I haz the power, 2016-10-25 23:00:42]

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

[RyanSturm, 2016-10-25 23:08:26]

jianli: PTAL

https://codereview.chromium.org/2362033002/diff/240001/chrome/android/java/sr...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://codereview.chromium.org/2362033002/diff/240001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:2883: public
boolean isShowingOfflinePreview() {
On 2016/10/25 22:14:16, jianli wrote:
> We should try to avoid adding more offline related codes into Tab. Instead,
you
> could put them in OfflinePageUtils (see how I did it for
> getOfflinePageHeaderForReload).
> 
> FYI, this is per feedback from tedchoc in my previous review. We're going to
> move offline handling logic from Tab to OfflinePageUtils.

Done.

https://codereview.chromium.org/2362033002/diff/240001/chrome/browser/android...
File chrome/browser/android/offline_pages/offline_page_tab_helper.cc (right):

https://codereview.chromium.org/2362033002/diff/240001/chrome/browser/android...
chrome/browser/android/offline_pages/offline_page_tab_helper.cc:181: }
On 2016/10/25 22:14:16, jianli wrote:
> nit: empty line

Done.

https://codereview.chromium.org/2362033002/diff/240001/chrome/browser/android...
File chrome/browser/android/offline_pages/offline_page_tab_helper.h (right):

https://codereview.chromium.org/2362033002/diff/240001/chrome/browser/android...
chrome/browser/android/offline_pages/offline_page_tab_helper.h:45: bool
IsOfflinePreview() const;
On 2016/10/25 22:14:16, jianli wrote:
> nit: rename to IsShowingOfflinePreview in order to be consistent with other
> places
> 
> Also, please change is_offline_preview to is_showing_offline_preview in
> LoadedOfflinePageInfo.

Done.

https://codereview.chromium.org/2362033002/diff/240001/chrome/browser/android...
File chrome/browser/android/offline_pages/offline_page_utils.h (right):

https://codereview.chromium.org/2362033002/diff/240001/chrome/browser/android...
chrome/browser/android/offline_pages/offline_page_utils.h:47: // Returns whether
the offline page corresponding to |web_contents| is part of
On 2016/10/25 22:14:16, jianli wrote:
> The comment is a bit hard to understand. How about:
>   // Returns true if the offline page is shown for previewing purpose.

Done.

[jianli, 2016-10-25 23:55:27]

lgtm

https://codereview.chromium.org/2362033002/diff/260001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java
(right):

https://codereview.chromium.org/2362033002/diff/260001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java:407:
* @return True if an offline preview is being shown.
nit: please add comment for webContents parameter

https://codereview.chromium.org/2362033002/diff/260001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageTabObserver.java
(right):

https://codereview.chromium.org/2362033002/diff/260001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageTabObserver.java:223:
|| OfflinePageUtils.isShowingOfflinePreview(tab) || !isConnected()
Please comment why we exclude offline preview case.

https://codereview.chromium.org/2362033002/diff/260001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java
(right):

https://codereview.chromium.org/2362033002/diff/260001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java:549:
* @return True if an offline preview is being shown.
nit: add comment for tab parameter

[commit-bot: I haz the power, 2016-10-26 00:20:11]

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

[commit-bot: I haz the power, 2016-10-26 00:20:12]

Dry run: This issue passed the CQ dry run.

[RyanSturm, 2016-10-26 18:26:25]

ryansturm@chromium.org changed reviewers:
+ bmcquade@chromium.org
- bengr@chromium.org, dimich@chromium.org

[RyanSturm, 2016-10-26 18:26:26]

removing bengr, and dimich

megjablon, bmcquade: PTAL

Thanks

[Bryan McQuade, 2016-10-26 18:30:02]

lgtm

[megjablon, 2016-10-26 22:01:11]

LGTM % comments

"The "show original" functionality is still pending, so if the
user is still on a prohibitively slow network the offline page will be
shown again." - Will this be changed before launch? Maybe add a comment where we
currently reload?

https://codereview.chromium.org/2362033002/diff/260001/chrome/browser/preview...
File chrome/browser/previews/previews_infobar_delegate.cc (right):

https://codereview.chromium.org/2362033002/diff/260001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_delegate.cc:124:
InfoBarService::WebContentsFromInfoBar(infobar());
Any reason why this was changed from auto*?

[RyanSturm, 2016-10-26 23:45:33]

Description was changed from

==========
Showing previews UI for Offline Previews

This changes the functionality of is_offline_preview() in OfflinePageTabHelper
to check the provisional information as well.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features. The "show original" functionality is still pending, so if the
user is still on a prohibitively slow network the offline page will be
shown again.

BUG=615564,649148
==========

to

==========
Showing previews UI for Offline Previews

This changes the functionality of is_offline_preview() in OfflinePageTabHelper
to check the provisional information as well.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features.

BUG=615564,649148
==========

[RyanSturm, 2016-10-27 00:01:02]

Thanks for the reviews.

> "The "show original" functionality is still pending, so if the
> user is still on a prohibitively slow network the offline page will be
> shown again." - Will this be changed before launch? Maybe add a comment where
we
> currently reload?

Removed this from the description, as this functionality just landed today.

https://codereview.chromium.org/2362033002/diff/260001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java
(right):

https://codereview.chromium.org/2362033002/diff/260001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java:407:
* @return True if an offline preview is being shown.
On 2016/10/25 23:55:27, jianli wrote:
> nit: please add comment for webContents parameter

Done.

https://codereview.chromium.org/2362033002/diff/260001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageTabObserver.java
(right):

https://codereview.chromium.org/2362033002/diff/260001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageTabObserver.java:223:
|| OfflinePageUtils.isShowingOfflinePreview(tab) || !isConnected()
On 2016/10/25 23:55:27, jianli wrote:
> Please comment why we exclude offline preview case.

Done.

https://codereview.chromium.org/2362033002/diff/260001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java
(right):

https://codereview.chromium.org/2362033002/diff/260001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java:549:
* @return True if an offline preview is being shown.
On 2016/10/25 23:55:27, jianli wrote:
> nit: add comment for tab parameter

Done.

https://codereview.chromium.org/2362033002/diff/260001/chrome/browser/preview...
File chrome/browser/previews/previews_infobar_delegate.cc (right):

https://codereview.chromium.org/2362033002/diff/260001/chrome/browser/preview...
chrome/browser/previews/previews_infobar_delegate.cc:124:
InfoBarService::WebContentsFromInfoBar(infobar());
On 2016/10/26 22:01:11, megjablon wrote:
> Any reason why this was changed from auto*?

It's unclear from the Style guide if auto is really allowed here, but I prefer
declaring types except for iterators. I think I added it for myself and forgot
to remove it.

[RyanSturm, 2016-10-27 00:02:57]

The CQ bit was checked by ryansturm@chromium.org

[RyanSturm, 2016-10-27 00:02:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmcquade@chromium.org, jianli@chromium.org, megjablon@chromium.org
Link to the patchset: https://codereview.chromium.org/2362033002/#ps280001
(title: "comments")

[commit-bot: I haz the power, 2016-10-27 00:03:21]

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, 2016-10-27 01:13:26]

Description was changed from

==========
Showing previews UI for Offline Previews

This changes the functionality of is_offline_preview() in OfflinePageTabHelper
to check the provisional information as well.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features.

BUG=615564,649148
==========

to

==========
Showing previews UI for Offline Previews

This changes the functionality of is_offline_preview() in OfflinePageTabHelper
to check the provisional information as well.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features.

BUG=615564,649148
==========

[commit-bot: I haz the power, 2016-10-27 01:13:28]

Committed patchset #15 (id:280001)

[commit-bot: I haz the power, 2016-10-27 01:16:01]

Description was changed from

==========
Showing previews UI for Offline Previews

This changes the functionality of is_offline_preview() in OfflinePageTabHelper
to check the provisional information as well.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features.

BUG=615564,649148
==========

to

==========
Showing previews UI for Offline Previews

This changes the functionality of is_offline_preview() in OfflinePageTabHelper
to check the provisional information as well.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features.

BUG=615564,649148

Committed: https://crrev.com/21364834a65e541369e8895d93695dfaf114056b
Cr-Commit-Position: refs/heads/master@{#427902}
==========

[commit-bot: I haz the power, 2016-10-27 01:16:03]

Patchset 15 (id:??) landed as
https://crrev.com/21364834a65e541369e8895d93695dfaf114056b
Cr-Commit-Position: refs/heads/master@{#427902}

[alancutter (OOO until 2018), 2016-10-27 04:23:53]

A revert of this CL (patchset #15 id:280001) has been created in
https://codereview.chromium.org/2446113008/ by alancutter@chromium.org.

The reason for reverting is: The following tests are failing on Android Tests:
org.chromium.chrome.browser.offlinepages.OfflinePageTabObserverTest#testOnConnectionTypeChanged_pageNotLoaded
org.chromium.chrome.browser.offlinepages.OfflinePageTabObserverTest#testOnDestroyed
org.chromium.chrome.browser.offlinepages.OfflinePageTabObserverTest#testOnPageLoadFinished_notConnected
org.chromium.chrome.browser.offlinepages.OfflinePageTabObserverTest#testOnPageLoadFinished
org.chromium.chrome.browser.offlinepages.OfflinePageTabObserverTest#testShowSnackbar_ignoreEventsAfterShownOnce
org.chromium.chrome.browser.offlinepages.OfflinePageTabObserverTest#testAddObserverForTab
org.chromium.chrome.browser.offlinepages.OfflinePageTabObserverTest#testOnUrlUpdated_whenSnackbarShown
org.chromium.chrome.browser.offlinepages.OfflinePageTabObserverTest#testOnHidden_afterSnackbarShown
org.chromium.chrome.browser.offlinepages.OfflinePageTabObserverTest#testOnConnectionTypeChanged_notConnected
org.chromium.chrome.browser.offlinepages.OfflinePageTabObserverTest#testShowSnackbar_onConnectionTypeChanged
org.chromium.chrome.browser.offlinepages.OfflinePageTabObserverTest#testAddObserverForTab_whenConnected
org.chromium.chrome.browser.offlinepages.OfflinePageTabObserverTest#testShowSnackbar_onShown
org.chromium.chrome.browser.offlinepages.OfflinePageTabObserverTest#testShowSnackbar_onPageLoadFinished

https://uberchromegw.corp.google.com/i/chromium.linux/builders/Android%20Test....

[RyanSturm, 2016-10-27 13:40:48]

Description was changed from

==========
Showing previews UI for Offline Previews

This changes the functionality of is_offline_preview() in OfflinePageTabHelper
to check the provisional information as well.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features.

BUG=615564,649148

Committed: https://crrev.com/21364834a65e541369e8895d93695dfaf114056b
Cr-Commit-Position: refs/heads/master@{#427902}
==========

to

==========
Showing previews UI for Offline Previews

This changes the functionality of is_offline_preview() in OfflinePageTabHelper
to check the provisional information as well.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features.

BUG=615564,649148

Committed: https://crrev.com/21364834a65e541369e8895d93695dfaf114056b
Cr-Commit-Position: refs/heads/master@{#427902}
==========

[RyanSturm, 2016-10-27 13:46:42]

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

[commit-bot: I haz the power, 2016-10-27 13:47: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, 2016-10-27 13:50:48]

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

[commit-bot: I haz the power, 2016-10-27 13:50:49]

Dry run: This issue passed the CQ dry run.

[RyanSturm, 2016-10-27 13:52:30]

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

[commit-bot: I haz the power, 2016-10-27 13:52:59]

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, 2016-10-27 13:56:24]

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

[commit-bot: I haz the power, 2016-10-27 13:56:26]

Dry run: This issue passed the CQ dry run.

[RyanSturm, 2016-10-27 13:57:11]

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

[commit-bot: I haz the power, 2016-10-27 13:57:31]

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

[RyanSturm, 2016-10-27 14:01:06]

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

[commit-bot: I haz the power, 2016-10-27 14:01:30]

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, 2016-10-27 14:45:04]

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

[commit-bot: I haz the power, 2016-10-27 14:45:05]

Dry run: This issue passed the CQ dry run.

[RyanSturm, 2016-10-27 18:28:28]

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

[commit-bot: I haz the power, 2016-10-27 18:29:12]

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

[RyanSturm, 2016-10-27 19:22:12]

ryansturm@chromium.org changed reviewers:
+ dewittj@chromium.org

[RyanSturm, 2016-10-27 19:22:14]

dewittj: PTAL @ changes to OfflinePageTabObserver.java and
OfflinePageTabObserverTest.java

[commit-bot: I haz the power, 2016-10-27 19:56:38]

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

[commit-bot: I haz the power, 2016-10-27 19:56:40]

Dry run: This issue passed the CQ dry run.

[dewittj, 2016-10-27 20:13:13]

lgtm

[RyanSturm, 2016-10-27 20:19:17]

The CQ bit was checked by ryansturm@chromium.org

[RyanSturm, 2016-10-27 20:19:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmcquade@chromium.org, jianli@chromium.org, megjablon@chromium.org
Link to the patchset: https://codereview.chromium.org/2362033002/#ps320001
(title: "java fixes for mocking methods")

[commit-bot: I haz the power, 2016-10-27 20:20:16]

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, 2016-10-27 20:27:07]

Description was changed from

==========
Showing previews UI for Offline Previews

This changes the functionality of is_offline_preview() in OfflinePageTabHelper
to check the provisional information as well.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features.

BUG=615564,649148

Committed: https://crrev.com/21364834a65e541369e8895d93695dfaf114056b
Cr-Commit-Position: refs/heads/master@{#427902}
==========

to

==========
Showing previews UI for Offline Previews

This changes the functionality of is_offline_preview() in OfflinePageTabHelper
to check the provisional information as well.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features.

BUG=615564,649148

Committed: https://crrev.com/21364834a65e541369e8895d93695dfaf114056b
Cr-Commit-Position: refs/heads/master@{#427902}
==========

[commit-bot: I haz the power, 2016-10-27 20:27:10]

Committed patchset #17 (id:320001)

[commit-bot: I haz the power, 2016-10-27 20:29:35]

Description was changed from

==========
Showing previews UI for Offline Previews

This changes the functionality of is_offline_preview() in OfflinePageTabHelper
to check the provisional information as well.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features.

BUG=615564,649148

Committed: https://crrev.com/21364834a65e541369e8895d93695dfaf114056b
Cr-Commit-Position: refs/heads/master@{#427902}
==========

to

==========
Showing previews UI for Offline Previews

This changes the functionality of is_offline_preview() in OfflinePageTabHelper
to check the provisional information as well.

Additionally, this CL addresses other consumers of this information that
want access to it in DidFinishNavigation. Specifically,
PreviewsPageLoadMetricsObserver will access the is_offline_previews bit
as will PreviewsInfoBarHelper.

This also prevents showing the offline pages snackbar and replaces it with
the previews infobar. This leaves the Offline omnibox and other UI
features.

BUG=615564,649148

Committed: https://crrev.com/21364834a65e541369e8895d93695dfaf114056b
Committed: https://crrev.com/e28ec590f2f8f5103fb724c587d3abc1a6782cb8
Cr-Original-Commit-Position: refs/heads/master@{#427902}
Cr-Commit-Position: refs/heads/master@{#428124}
==========

[commit-bot: I haz the power, 2016-10-27 20:29:37]

Patchset 17 (id:??) landed as
https://crrev.com/e28ec590f2f8f5103fb724c587d3abc1a6782cb8
Cr-Commit-Position: refs/heads/master@{#428124}

[phoglund_chromium, 2016-10-28 09:39:26]

A revert of this CL (patchset #17 id:320001) has been created in
https://codereview.chromium.org/2450353005/ by phoglund@chromium.org.

The reason for reverting is: Speculative revert: might have broken a bunch of
infobar tests. This CL is the only infobar-related one in the blamelist. 
https://uberchromegw.corp.google.com/i/chromium.linux/builders/Android%20Test...

For instance
org.chromium.chrome.browser.infobar.InfoBarContainerTest#testCloseTabOnAdd:

junit.framework.AssertionFailedError: expected:<2> but was:<1>
	at
org.chromium.chrome.browser.infobar.InfoBarContainerTest.addInfoBarToCurrentTab(InfoBarContainerTest.java:103)
	at
org.chromium.chrome.browser.infobar.InfoBarContainerTest.testCloseTabOnAdd(InfoBarContainerTest.java:223).