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

Issue 667722 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

PhysicalWebDataSourceAndroid returns overflowed timestamps.

Project Member Reported by vitaliii@chromium.org, Nov 22 2016

Issue description

Version: ToT
OS: Android

|GetMetadata| is bridged from java and here
https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java?type=cs&q=%22(int)+urlInfo.getScanTimestamp()%22&sq=package:chromium&l=297

the milliseconds timestamp (from |System.currentTimeMillis()|) is casted to int.

Please propagate long instead.

 
Blocking: 666647
This a launch blocker for us.
I would be grateful if we could have this done well by 6th of January (M57 FF).

Comment 3 by cco3@chromium.org, Dec 6 2016

Owner: cco3@chromium.org

Comment 4 by cco3@chromium.org, Dec 6 2016

Status: Started (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 13 2016

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

commit 343cfde27b2dc5779a5cff12f0d511b57e3b3deb
Author: cco3 <cco3@chromium.org>
Date: Tue Dec 13 22:26:35 2016

Use GURLs in Physical Web data source

The Physical Web data source passes URLs to listeners, currently in the
form of strings.  This change passes them around as GURLs.

BUG= 667722 

Review-Url: https://codereview.chromium.org/2565023002
Cr-Commit-Position: refs/heads/master@{#438313}

[modify] https://crrev.com/343cfde27b2dc5779a5cff12f0d511b57e3b3deb/chrome/browser/android/physical_web/physical_web_data_source_android.cc
[modify] https://crrev.com/343cfde27b2dc5779a5cff12f0d511b57e3b3deb/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.cc
[modify] https://crrev.com/343cfde27b2dc5779a5cff12f0d511b57e3b3deb/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.h
[modify] https://crrev.com/343cfde27b2dc5779a5cff12f0d511b57e3b3deb/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider_unittest.cc
[modify] https://crrev.com/343cfde27b2dc5779a5cff12f0d511b57e3b3deb/components/physical_web/data_source/BUILD.gn
[modify] https://crrev.com/343cfde27b2dc5779a5cff12f0d511b57e3b3deb/components/physical_web/data_source/fake_physical_web_data_source.cc
[modify] https://crrev.com/343cfde27b2dc5779a5cff12f0d511b57e3b3deb/components/physical_web/data_source/fake_physical_web_data_source.h
[modify] https://crrev.com/343cfde27b2dc5779a5cff12f0d511b57e3b3deb/components/physical_web/data_source/physical_web_data_source_impl.cc
[modify] https://crrev.com/343cfde27b2dc5779a5cff12f0d511b57e3b3deb/components/physical_web/data_source/physical_web_data_source_impl.h
[modify] https://crrev.com/343cfde27b2dc5779a5cff12f0d511b57e3b3deb/components/physical_web/data_source/physical_web_data_source_impl_unittest.cc
[modify] https://crrev.com/343cfde27b2dc5779a5cff12f0d511b57e3b3deb/components/physical_web/data_source/physical_web_listener.h

Blocking: -666647
Labels: -Pri-1 Pri-2
We decided not to show time at all, so this is *not* a launch blocker for us anymore. However, I like pending |struct| change a lot. I even have a bug for it ( issue 668141 ), please add it to your CL.

Comment 7 by cco3@chromium.org, Dec 16 2016

Will do.
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 13 2017

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

commit 4417ce64c063776af45d1d897f15cdc0be361a2e
Author: cco3 <cco3@chromium.org>
Date: Fri Jan 13 01:09:36 2017

Pass Physical Web metadata through a struct

Currently, we use DictionaryValues in the Physical Web data source.
The problem with this is that we cannot store store longs (timestamps).
This change introduces a metadata struct that can be requested from the
data source, deprecating the DictionaryValue option.

BUG= 667722 

Review-Url: https://codereview.chromium.org/2561493002
Cr-Commit-Position: refs/heads/master@{#443425}

[modify] https://crrev.com/4417ce64c063776af45d1d897f15cdc0be361a2e/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java
[modify] https://crrev.com/4417ce64c063776af45d1d897f15cdc0be361a2e/chrome/browser/android/physical_web/physical_web_data_source_android.cc
[modify] https://crrev.com/4417ce64c063776af45d1d897f15cdc0be361a2e/chrome/browser/android/physical_web/physical_web_data_source_android.h
[add] https://crrev.com/4417ce64c063776af45d1d897f15cdc0be361a2e/chrome/browser/android/physical_web/physical_web_data_source_android_unittest.cc
[modify] https://crrev.com/4417ce64c063776af45d1d897f15cdc0be361a2e/chrome/test/BUILD.gn
[modify] https://crrev.com/4417ce64c063776af45d1d897f15cdc0be361a2e/components/physical_web/data_source/BUILD.gn
[modify] https://crrev.com/4417ce64c063776af45d1d897f15cdc0be361a2e/components/physical_web/data_source/fake_physical_web_data_source.cc
[modify] https://crrev.com/4417ce64c063776af45d1d897f15cdc0be361a2e/components/physical_web/data_source/fake_physical_web_data_source.h
[modify] https://crrev.com/4417ce64c063776af45d1d897f15cdc0be361a2e/components/physical_web/data_source/physical_web_data_source.cc
[modify] https://crrev.com/4417ce64c063776af45d1d897f15cdc0be361a2e/components/physical_web/data_source/physical_web_data_source.h
[modify] https://crrev.com/4417ce64c063776af45d1d897f15cdc0be361a2e/components/physical_web/data_source/physical_web_data_source_impl_unittest.cc
[modify] https://crrev.com/4417ce64c063776af45d1d897f15cdc0be361a2e/ios/chrome/browser/physical_web/ios_chrome_physical_web_data_source.h
[modify] https://crrev.com/4417ce64c063776af45d1d897f15cdc0be361a2e/ios/chrome/browser/physical_web/ios_chrome_physical_web_data_source.mm
[modify] https://crrev.com/4417ce64c063776af45d1d897f15cdc0be361a2e/ios/chrome/common/physical_web/physical_web_scanner.h
[modify] https://crrev.com/4417ce64c063776af45d1d897f15cdc0be361a2e/ios/chrome/common/physical_web/physical_web_scanner.mm

Status: Fixed (was: Started)
Project Member

Comment 12 by bugdroid1@chromium.org, Jan 25 2017

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

commit e35ee04f4a71c990999822ede670b3f4ed51535d
Author: mattreynolds <mattreynolds@chromium.org>
Date: Wed Jan 25 00:17:23 2017

Remove GetMetadata from PhysicalWebDataSource and PhysicalWebScanner

GetMetadata has been deprecated in favor of GetMetadataList. All previous
uses of GetMetadata have been switched, so let's remove GetMetadata.

BUG= 667722 

Review-Url: https://codereview.chromium.org/2648973004
Cr-Commit-Position: refs/heads/master@{#445872}

[modify] https://crrev.com/e35ee04f4a71c990999822ede670b3f4ed51535d/chrome/browser/android/physical_web/physical_web_data_source_android.cc
[modify] https://crrev.com/e35ee04f4a71c990999822ede670b3f4ed51535d/chrome/browser/android/physical_web/physical_web_data_source_android.h
[modify] https://crrev.com/e35ee04f4a71c990999822ede670b3f4ed51535d/chrome/browser/android/physical_web/physical_web_data_source_android_unittest.cc
[modify] https://crrev.com/e35ee04f4a71c990999822ede670b3f4ed51535d/components/physical_web/data_source/fake_physical_web_data_source.cc
[modify] https://crrev.com/e35ee04f4a71c990999822ede670b3f4ed51535d/components/physical_web/data_source/fake_physical_web_data_source.h
[modify] https://crrev.com/e35ee04f4a71c990999822ede670b3f4ed51535d/components/physical_web/data_source/physical_web_data_source.cc
[modify] https://crrev.com/e35ee04f4a71c990999822ede670b3f4ed51535d/components/physical_web/data_source/physical_web_data_source.h
[modify] https://crrev.com/e35ee04f4a71c990999822ede670b3f4ed51535d/components/physical_web/data_source/physical_web_data_source_impl_unittest.cc
[modify] https://crrev.com/e35ee04f4a71c990999822ede670b3f4ed51535d/ios/chrome/browser/physical_web/ios_chrome_physical_web_data_source.h
[modify] https://crrev.com/e35ee04f4a71c990999822ede670b3f4ed51535d/ios/chrome/browser/physical_web/ios_chrome_physical_web_data_source.mm
[modify] https://crrev.com/e35ee04f4a71c990999822ede670b3f4ed51535d/ios/chrome/common/physical_web/physical_web_scanner.h
[modify] https://crrev.com/e35ee04f4a71c990999822ede670b3f4ed51535d/ios/chrome/common/physical_web/physical_web_scanner.mm

Sign in to add a comment