Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(422)

Issue 18755003: Fix the allHistory flag on chrome.history.onVisitRemoved event. (Closed)

Created:
7 years, 5 months ago by msimonides
Modified:
7 years, 5 months ago
Reviewers:
brettw, Matt Perry
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Fix the allHistory flag on chrome.history.onVisitRemoved event. Calling chrome.history.deleteAll results in chrome.history.onVisitRemoved events to be dispatched with the allHistory flag being false instead of true. This fix changes the call to ExpireHistoryBetween so that begin_time and end_time are null which results in the history backend using a special path to delete all history and dispatch the right event. BUG=258445 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=211376

Patch Set 1 #

Patch Set 2 : Fix incorrect direct constructor call. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
M chrome/browser/extensions/api/history/history_api.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
msimonides
I have made the fix in the history API but I wonder if the issue ...
7 years, 5 months ago (2013-07-08 07:34:36 UTC) #1
Matt Perry
The change seems unrelated to the CL description. Also, is there a bug entry for ...
7 years, 5 months ago (2013-07-08 21:18:51 UTC) #2
msimonides
On 2013/07/08 21:18:51, Matt Perry wrote: > The change seems unrelated to the CL description. ...
7 years, 5 months ago (2013-07-09 13:53:39 UTC) #3
Matt Perry
Makes sense! Thanks for the explanation. Please change the BUG= line to BUG=<bug number>. Then ...
7 years, 5 months ago (2013-07-09 18:11:07 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msimonides@opera.com/18755003/1
7 years, 5 months ago (2013-07-10 07:02:46 UTC) #5
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 5 months ago (2013-07-10 07:36:41 UTC) #6
msimonides
Added a compilation fixup
7 years, 5 months ago (2013-07-11 07:50:21 UTC) #7
msimonides
On 2013/07/11 07:50:21, msimonides wrote: > Added a compilation fixup Looks like I cannot run ...
7 years, 5 months ago (2013-07-11 09:36:17 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msimonides@opera.com/18755003/26001
7 years, 5 months ago (2013-07-12 07:47:24 UTC) #9
commit-bot: I haz the power
7 years, 5 months ago (2013-07-12 11:05:01 UTC) #10
Message was sent while issue was closed.
Change committed as 211376

Powered by Google App Engine
This is Rietveld 408576698