New issue
Advanced search Search tips

Issue 778416 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug
q4

Blocking:
issue 900667



Sign in to add a comment

Move voice search tab logic entirely into tab helper

Project Member Reported by kkhorimoto@chromium.org, Oct 25 2017

Issue description

crrev.com/c/726842 is a temporary solution that splits the behavior of Tab's |isVoiceSearchResultsTab| between Tab and a helper object.  This was done in order to keep the change small, since it needs to be cherry-picked.  However, the long-term solution will be to audit callers of |isVoiceSearchResultsTab| and update them to reference an observer API for the tab helper object.
 
Components: UI>Browser>Search>Voice
Owner: mrefaat@chromium.org
Will remove this method and clean up todos related to it as part of tab refactoring.
Labels: q4 S-ios-voice-search ios-tab-refactoring small O-ios-tab-refactoring-extras
Blocking: 900667
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 28

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

commit 3eea2e46ea1df609f21ab9169d1bafe9e709436d
Author: mrefaat <mrefaat@chromium.org>
Date: Wed Nov 28 22:53:58 2018

Remove isVoiceSearchResultsTab from tab

This was created for voicesearch bar which is removed with the new UI.

Bug:  898697 ,  778416 
Change-Id: Ice43ef892a886bf616f8bc2712c0a6b2d48b29f8
Reviewed-on: https://chromium-review.googlesource.com/c/1353936
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611914}
[modify] https://crrev.com/3eea2e46ea1df609f21ab9169d1bafe9e709436d/ios/chrome/browser/tabs/tab.h
[modify] https://crrev.com/3eea2e46ea1df609f21ab9169d1bafe9e709436d/ios/chrome/browser/tabs/tab.mm

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 28

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

commit d4c771eb958a58e86e0113a65974210079375d15
Author: mrefaat <mrefaat@chromium.org>
Date: Wed Nov 28 23:55:20 2018

Remove ordering restriction for VoiceSearchTabHelper creation

Tab no longer use the VoiceSearchTabHelper, so ordering is not needed
any more

Bug:  778416 
Change-Id: I37a84f76de97be5af8956eb829808f86ac839468
Reviewed-on: https://chromium-review.googlesource.com/c/1353937
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611943}
[modify] https://crrev.com/d4c771eb958a58e86e0113a65974210079375d15/ios/chrome/browser/tabs/tab_helper_util.mm

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 29

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

commit 2be9629437bc9c63418ad17d71bc083a73672bc0
Author: mrefaat <mrefaat@chromium.org>
Date: Thu Nov 29 23:43:09 2018

Remove isVoiceSearchNavigationItem from voice_search_navigations_tab_helper

With voice search bar removal and the tab cleanup this method is not used anymore
deleted it and updated unittests.


Bug:  778416 
Change-Id: Ia6a130f50eafb01b0eef265e98ca412071ba7b95
Reviewed-on: https://chromium-review.googlesource.com/c/1355765
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612436}
[modify] https://crrev.com/2be9629437bc9c63418ad17d71bc083a73672bc0/ios/chrome/browser/voice/voice_search_navigations_tab_helper.h
[modify] https://crrev.com/2be9629437bc9c63418ad17d71bc083a73672bc0/ios/chrome/browser/voice/voice_search_navigations_tab_helper.mm
[modify] https://crrev.com/2be9629437bc9c63418ad17d71bc083a73672bc0/ios/chrome/browser/voice/voice_search_navigations_tab_helper_unittest.mm

Status: Fixed (was: Assigned)

Sign in to add a comment