New issue
Advanced search Search tips

Issue 901563 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Task



Sign in to add a comment

Remove WebState::ShowTransientContentView

Project Member Reported by eugene...@chromium.org, Nov 3

Issue description

This API is only used by SadTab. SadTabView should be presented by SadTabCoordinator in SadTabViewController.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0573cab67d14d0c8fed5403f0be7d059df400fe3

commit 0573cab67d14d0c8fed5403f0be7d059df400fe3
Author: Eugene But <eugenebut@google.com>
Date: Tue Nov 06 00:20:40 2018

Create SadTabViewController which owns SadTabView.

Changes to SadTabView:
 - messageTextView and actionButton are now public
   for testing (http://go/unit-test-practices#public-apis)
 - _messageTextView and _actionButton ivars are not
   synthesized anymore, because properties belong to a
   category

SadTabViewController will be owned by SadTabCoordinator
which will be landed in a follow up CL. Using view controller
for UI presentation will allow to avoid using Transient View
for Sad Tab and will follow New Architecture patterns.

Bug: 901563
Change-Id: If84d89e9bc3d991e743f3420406881292b345579
Reviewed-on: https://chromium-review.googlesource.com/c/1312684
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605525}
[modify] https://crrev.com/0573cab67d14d0c8fed5403f0be7d059df400fe3/ios/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/0573cab67d14d0c8fed5403f0be7d059df400fe3/ios/chrome/browser/ui/sad_tab/BUILD.gn
[modify] https://crrev.com/0573cab67d14d0c8fed5403f0be7d059df400fe3/ios/chrome/browser/ui/sad_tab/sad_tab_view.h
[modify] https://crrev.com/0573cab67d14d0c8fed5403f0be7d059df400fe3/ios/chrome/browser/ui/sad_tab/sad_tab_view.mm
[add] https://crrev.com/0573cab67d14d0c8fed5403f0be7d059df400fe3/ios/chrome/browser/ui/sad_tab/sad_tab_view_controller.h
[add] https://crrev.com/0573cab67d14d0c8fed5403f0be7d059df400fe3/ios/chrome/browser/ui/sad_tab/sad_tab_view_controller.mm
[add] https://crrev.com/0573cab67d14d0c8fed5403f0be7d059df400fe3/ios/chrome/browser/ui/sad_tab/sad_tab_view_controller_unittest.mm
[modify] https://crrev.com/0573cab67d14d0c8fed5403f0be7d059df400fe3/ios/chrome/test/BUILD.gn

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2278b7f5b52affb8f2a4abc9b5ab7ad46736d4c6

commit 2278b7f5b52affb8f2a4abc9b5ab7ad46736d4c6
Author: Eugene But <eugenebut@google.com>
Date: Thu Nov 08 03:37:46 2018

Create SadTabCoordinator which owns SadTabViewController.

SadTabCoordinator will be used instead SadTabLegacyCoordinator
for SadTab UI presentation. SadTabLegacyCoordinator does not
use view controller and presents SadTab UI with
WebState::ShowTransientView, which is misuse of WebState API.

Bug: 901563

Change-Id: I7c29c82d0036c05d06e44e8806ff0b82adad34ca
Reviewed-on: https://chromium-review.googlesource.com/c/1320593
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606321}
[modify] https://crrev.com/2278b7f5b52affb8f2a4abc9b5ab7ad46736d4c6/ios/chrome/browser/ui/sad_tab/BUILD.gn
[add] https://crrev.com/2278b7f5b52affb8f2a4abc9b5ab7ad46736d4c6/ios/chrome/browser/ui/sad_tab/sad_tab_coordinator.h
[add] https://crrev.com/2278b7f5b52affb8f2a4abc9b5ab7ad46736d4c6/ios/chrome/browser/ui/sad_tab/sad_tab_coordinator.mm
[add] https://crrev.com/2278b7f5b52affb8f2a4abc9b5ab7ad46736d4c6/ios/chrome/browser/ui/sad_tab/sad_tab_coordinator_unittest.mm
[modify] https://crrev.com/2278b7f5b52affb8f2a4abc9b5ab7ad46736d4c6/ios/chrome/browser/ui/sad_tab/sad_tab_legacy_coordinator.mm
[modify] https://crrev.com/2278b7f5b52affb8f2a4abc9b5ab7ad46736d4c6/ios/chrome/browser/web/sad_tab_tab_helper_delegate.h
[modify] https://crrev.com/2278b7f5b52affb8f2a4abc9b5ab7ad46736d4c6/ios/chrome/browser/web/sad_tab_tab_helper_unittest.mm

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9773a84184a277ad674bbe729aa26677f1f3c800

commit 9773a84184a277ad674bbe729aa26677f1f3c800
Author: Eugene But <eugenebut@google.com>
Date: Fri Nov 09 15:56:41 2018

Update SadTabTabHelper to work with SadTabCoordinator.

SadTabTabHelper now calls sadTabTabHelperDidHide: and
sadTabTabHelper:didShowForRepeatedFailure:

Bug: 901563
Change-Id: If2b75702c951c247d44368b681ce360bbb6464d5
Reviewed-on: https://chromium-review.googlesource.com/c/1327167
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606854}
[modify] https://crrev.com/9773a84184a277ad674bbe729aa26677f1f3c800/ios/chrome/browser/web/sad_tab_tab_helper.h
[modify] https://crrev.com/9773a84184a277ad674bbe729aa26677f1f3c800/ios/chrome/browser/web/sad_tab_tab_helper.mm
[modify] https://crrev.com/9773a84184a277ad674bbe729aa26677f1f3c800/ios/chrome/browser/web/sad_tab_tab_helper_unittest.mm

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/715e6d8b7afb9e2764f700fd444ca41937fcda50

commit 715e6d8b7afb9e2764f700fd444ca41937fcda50
Author: Eugene But <eugenebut@google.com>
Date: Tue Nov 13 00:21:53 2018

Integrate SadTabCoordinator into BVC.

Now SadTabCoordinator will be used instead SadTabLegacyCoordinator
for SadTab UI presentation. SadTabLegacyCoordinator does not
use view controller and presents SadTab UI with
WebState::ShowTransientView, which is misuse of WebState API.

SadTabCoordinator is disabled by default and can be enabled via
kPresentSadTabInViewController feature flag.

Bug: 901563
Change-Id: I34dd6ae99255e5fe0b3ef100265c78169d619a50
Reviewed-on: https://chromium-review.googlesource.com/c/1329924
Commit-Queue: Eugene But <eugenebut@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607386}
[modify] https://crrev.com/715e6d8b7afb9e2764f700fd444ca41937fcda50/ios/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/715e6d8b7afb9e2764f700fd444ca41937fcda50/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/715e6d8b7afb9e2764f700fd444ca41937fcda50/ios/chrome/browser/ui/side_swipe/side_swipe_controller.h
[modify] https://crrev.com/715e6d8b7afb9e2764f700fd444ca41937fcda50/ios/chrome/browser/ui/side_swipe/side_swipe_controller.mm
[modify] https://crrev.com/715e6d8b7afb9e2764f700fd444ca41937fcda50/ios/chrome/browser/web/sad_tab_tab_helper.h
[modify] https://crrev.com/715e6d8b7afb9e2764f700fd444ca41937fcda50/ios/chrome/browser/web/sad_tab_tab_helper_unittest.mm

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b7b1438389c340c1b47410485d3aa770276b0e38

commit b7b1438389c340c1b47410485d3aa770276b0e38
Author: Eugene But <eugenebut@google.com>
Date: Tue Nov 13 22:45:47 2018

Add Overscroll Actions support to SadTabViewController.

This CL adds OverscrollActionsController to SadTabViewController.
OverscrollActionsController requires UIScrollView to work, so
SadTabViewController creates a dummy scroll view to hold SadTabView.

Bug: 901563
Change-Id: I564ed10b807d774b891d86c21cda58435a5681c2
Reviewed-on: https://chromium-review.googlesource.com/c/1332390
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607780}
[modify] https://crrev.com/b7b1438389c340c1b47410485d3aa770276b0e38/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/b7b1438389c340c1b47410485d3aa770276b0e38/ios/chrome/browser/ui/sad_tab/BUILD.gn
[modify] https://crrev.com/b7b1438389c340c1b47410485d3aa770276b0e38/ios/chrome/browser/ui/sad_tab/sad_tab_coordinator.h
[modify] https://crrev.com/b7b1438389c340c1b47410485d3aa770276b0e38/ios/chrome/browser/ui/sad_tab/sad_tab_coordinator.mm
[modify] https://crrev.com/b7b1438389c340c1b47410485d3aa770276b0e38/ios/chrome/browser/ui/sad_tab/sad_tab_view_controller.h
[modify] https://crrev.com/b7b1438389c340c1b47410485d3aa770276b0e38/ios/chrome/browser/ui/sad_tab/sad_tab_view_controller.mm

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/99162f9e29b36c069d244c6f5dc432674504085b

commit 99162f9e29b36c069d244c6f5dc432674504085b
Author: Eugene But <eugenebut@chromium.org>
Date: Fri Nov 16 22:56:49 2018

Do not start SadTabCoordinator for invisible Tab.

This fixes the issue when side swiping from one SadTab to another.
Since -[SadTabCoordinator start] is idempotent it is important to start
coordinator for new sad tab, not for the old one which is not visible
anymore.

Bug: 901563
Change-Id: I9dfb488037da18be5fdb1079e849b723fa723c08
Reviewed-on: https://chromium-review.googlesource.com/c/1337814
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609011}
[modify] https://crrev.com/99162f9e29b36c069d244c6f5dc432674504085b/ios/chrome/browser/ui/browser_view_controller.mm

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e962e12577ea63d505606b4e569453e5f0cbe440

commit e962e12577ea63d505606b4e569453e5f0cbe440
Author: Eugene But <eugenebut@google.com>
Date: Mon Nov 19 17:43:33 2018

Enable PresentSadTabInViewController feature by default.

Bug: 901563
Test:
 - SadTab is correctly presented for renderer crashes and chrome://crash
 - SadTab support pull to action
 - SadTab show different help text for incognito
 - SadTab bottons and links work as expected (reload, help page,
   feedback)
 - SadTab is correctly displayed in TabGrid and during SideSwipe
 - The user can switch between tabs and that does not break SadTab UI

Change-Id: I68a7a972a97375f4aff084aa83095a736c8e3b41
Reviewed-on: https://chromium-review.googlesource.com/c/1332775
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609349}
[modify] https://crrev.com/e962e12577ea63d505606b4e569453e5f0cbe440/ios/chrome/browser/ui/sad_tab/features.cc

Labels: M-74
Status: Assigned (was: Started)
Can be finished after shipping M72 and branching M73.

Sign in to add a comment