New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 716957 link

Starred by 1 user
Status: Fixed
Owner:
█ ortuno@chromium.org
Last visit > 30 days ago
Closed: Jun 2017
Cc:
█ mgiuca@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Chrome
Pri: 3
Type: Bug



Sign in to add a comment

ShareServiceImpl refactor

Project Member Reported by mgiuca@chromium.org, May 1 2017 
A small refactor on ShareServiceImpl class for Web Share on Desktop.

- Need an abstraction over reading pref store (in share_target_pref_helper).
- Create a ShareTarget class, parse it up in pref_helper and return that, instead of passing around a dictionary through ShareServiceImpl.

Comment 1 by ortuno@chromium.org, Jun 7 2017

Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 16 2017 

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

commit 97f8bd52b80fb4847af24c4a8a4a424da737716a
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Fri Jun 16 05:54:20 2017

webshare-target: Introduce ShareTarget class

1. Introduces a move-only ShareTarget class.
2. Refactor ShareServiceImpl to use ShareTarget.
  3.1. Pass a ShareTarget vector to ShowPicker.
  3.2. Modify picker's callback to return a ShareTarget. ShareTarget includes
       the URL template so we avoid an extra dictionary lookup when it returns.
4. Fix tests.
  4.1. Add PickTarget function to ShareServiceImplUnittest that picks a target
       based on the URL.

Bug:  716957 ,  717323 

Change-Id: If28d0a7576644f1501afdf9441c2f8845d9935d6
Reviewed-on: https://chromium-review.googlesource.com/517614
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479961}
[modify] https://crrev.com/97f8bd52b80fb4847af24c4a8a4a424da737716a/chrome/browser/BUILD.gn
[modify] https://crrev.com/97f8bd52b80fb4847af24c4a8a4a424da737716a/chrome/browser/ui/browser_dialogs.h
[modify] https://crrev.com/97f8bd52b80fb4847af24c4a8a4a424da737716a/chrome/browser/ui/views/webshare/webshare_target_picker_view.cc
[modify] https://crrev.com/97f8bd52b80fb4847af24c4a8a4a424da737716a/chrome/browser/ui/views/webshare/webshare_target_picker_view.h
[modify] https://crrev.com/97f8bd52b80fb4847af24c4a8a4a424da737716a/chrome/browser/ui/views/webshare/webshare_target_picker_view_unittest.cc
[modify] https://crrev.com/97f8bd52b80fb4847af24c4a8a4a424da737716a/chrome/browser/webshare/share_service_impl.cc
[modify] https://crrev.com/97f8bd52b80fb4847af24c4a8a4a424da737716a/chrome/browser/webshare/share_service_impl.h
[modify] https://crrev.com/97f8bd52b80fb4847af24c4a8a4a424da737716a/chrome/browser/webshare/share_service_impl_unittest.cc
[add] https://crrev.com/97f8bd52b80fb4847af24c4a8a4a424da737716a/chrome/browser/webshare/webshare_target.cc
[add] https://crrev.com/97f8bd52b80fb4847af24c4a8a4a424da737716a/chrome/browser/webshare/webshare_target.h

Comment 3 by ortuno@chromium.org, Jun 16 2017

Status: Fixed (was: Started)
►

Sign in to add a comment