Only show the notice about other forms of browsing history if sync is active

Only show the notice about other forms of browsing history if sync is active

We currently show the notice if Sync service is present, but that does not
imply that the user is actually syncing. The Sync service must also be
active.

Furthermore, the notice is to be shown to users without a custom passphrase,
not with it.

BUG=595332
Committed: https://crrev.com/7bbc6599a41fb032cb58a8defcec1fc22af42d35
Cr-Commit-Position: refs/heads/master@{#381960}

[msramek, 2016-03-18 09:35:55]

msramek@chromium.org changed reviewers:
+ msarda@chromium.org

[msramek, 2016-03-18 09:35:55]

Hi Mihai,

please have a look!

Thanks,
Martin

[msarda, 2016-03-18 13:51:25]

LGTM with a question of the previous code.

https://codereview.chromium.org/1814173002/diff/1/components/browsing_data_ui...
File components/browsing_data_ui/history_notice_utils.cc (right):

https://codereview.chromium.org/1814173002/diff/1/components/browsing_data_ui...
components/browsing_data_ui/history_notice_utils.cc:17:
sync_service->IsUsingSecondaryPassphrase() &&
I just want to make sure I understand: from the internal design document you say
that the other forms of browsing data should be shown when there is "no custom
passphrase". Should this be !sync_service->IsUsingSecondaryPassphrase()?

[msramek, 2016-03-18 14:00:58]

Description was changed from

==========
Only show the notice about other forms of browsing history if sync is active

We currently show the notice if Sync service is present, but that does not
imply that the user is actually syncing. The Sync service must also be
active.

BUG=595332
==========

to

==========
Only show the notice about other forms of browsing history if sync is active

We currently show the notice if Sync service is present, but that does not
imply that the user is actually syncing. The Sync service must also be
active.

Furthermore, the notice is to be shown to users without a custom passphrase,
not with it.

BUG=595332
==========

[msramek, 2016-03-18 14:01:58]

https://codereview.chromium.org/1814173002/diff/1/components/browsing_data_ui...
File components/browsing_data_ui/history_notice_utils.cc (right):

https://codereview.chromium.org/1814173002/diff/1/components/browsing_data_ui...
components/browsing_data_ui/history_notice_utils.cc:17:
sync_service->IsUsingSecondaryPassphrase() &&
On 2016/03/18 13:51:25, msarda wrote:
> I just want to make sure I understand: from the internal design document you
say
> that the other forms of browsing data should be shown when there is "no custom
> passphrase". Should this be !sync_service->IsUsingSecondaryPassphrase()?

Ah, yes. Thanks for catching this!

I updated the CL description, and will write a unittest in a followup CL.

[msramek, 2016-03-18 14:02:16]

The CQ bit was checked by msramek@chromium.org

[msramek, 2016-03-18 14:02:16]

The patchset sent to the CQ was uploaded after l-g-t-m from msarda@chromium.org
Link to the patchset: https://codereview.chromium.org/1814173002/#ps2 (title:
"Forgotten negation.")

[commit-bot: I haz the power, 2016-03-18 14:02:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1814173002/2
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1814173002/2

[commit-bot: I haz the power, 2016-03-18 14:34:36]

Description was changed from

==========
Only show the notice about other forms of browsing history if sync is active

We currently show the notice if Sync service is present, but that does not
imply that the user is actually syncing. The Sync service must also be
active.

Furthermore, the notice is to be shown to users without a custom passphrase,
not with it.

BUG=595332
==========

to

==========
Only show the notice about other forms of browsing history if sync is active

We currently show the notice if Sync service is present, but that does not
imply that the user is actually syncing. The Sync service must also be
active.

Furthermore, the notice is to be shown to users without a custom passphrase,
not with it.

BUG=595332
==========

[commit-bot: I haz the power, 2016-03-18 14:34:37]

Committed patchset #2 (id:2)

[commit-bot: I haz the power, 2016-03-18 14:35:36]

Description was changed from

==========
Only show the notice about other forms of browsing history if sync is active

We currently show the notice if Sync service is present, but that does not
imply that the user is actually syncing. The Sync service must also be
active.

Furthermore, the notice is to be shown to users without a custom passphrase,
not with it.

BUG=595332
==========

to

==========
Only show the notice about other forms of browsing history if sync is active

We currently show the notice if Sync service is present, but that does not
imply that the user is actually syncing. The Sync service must also be
active.

Furthermore, the notice is to be shown to users without a custom passphrase,
not with it.

BUG=595332

Committed: https://crrev.com/7bbc6599a41fb032cb58a8defcec1fc22af42d35
Cr-Commit-Position: refs/heads/master@{#381960}
==========

[commit-bot: I haz the power, 2016-03-18 14:35:37]

Patchset 2 (id:??) landed as
https://crrev.com/7bbc6599a41fb032cb58a8defcec1fc22af42d35
Cr-Commit-Position: refs/heads/master@{#381960}