The canonical bug format is TODO(crbug.com/<id>). TODOs of the
following forms will all be migrated to the new format:
- TODO(crbug.com/<old id>)
- TODO(https://crbug.com/<old id>)
- TODO(crbug/<old id>)
- TODO(crbug/monorail/<old id>)
- TODO(<old id>)
- TODO(issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/u/1/issues/<old id>)
- TODO(bugs.chromium.org/<old id>)
Bug id mapping is sourced from go/chrome-on-buganizer-prod-issues.
See go/crbug-todo-migration for details.
#crbug-todo-migration
Bug: b/321899722
Change-Id: Ibc66b8c440e4bcdef414e77fef4d9874d2ea9951
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5493800
Auto-Submit: Alison Gale <agale@chromium.org>
Commit-Queue: Alison Gale <agale@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Owners-Override: Alison Gale <agale@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1293330}
The canonical bug format is TODO(crbug.com/<id>). TODOs of the
following forms will all be migrated to the new format:
- TODO(crbug.com/<old id>)
- TODO(https://crbug.com/<old id>)
- TODO(crbug/<old id>)
- TODO(crbug/monorail/<old id>)
- TODO(<old id>)
- TODO(issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/u/1/issues/<old id>)
- TODO(bugs.chromium.org/<old id>)
Bug id mapping is sourced from go/chrome-on-buganizer-prod-issues.
See go/crbug-todo-migration for details.
#crbug-todo-migration
Bug: b/321899722
Change-Id: Ib028de8bb63c99e5a81d90e24e422cf88061ad05
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5469583
Owners-Override: Alison Gale <agale@chromium.org>
Reviewed-by: Darryl James <dljames@chromium.org>
Commit-Queue: Alison Gale <agale@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1290952}
The canonical bug format is TODO(crbug.com/<id>). TODOs of the
following forms will all be migrated to the new format:
- TODO(crbug.com/<old id>)
- TODO(https://crbug.com/<old id>)
- TODO(crbug/<old id>)
- TODO(crbug/monorail/<old id>)
- TODO(<old id>)
- TODO(issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/u/1/issues/<old id>)
- TODO(bugs.chromium.org/<old id>)
Bug id mapping is sourced from go/chrome-on-buganizer-prod-issues.
See go/crbug-todo-migration for details.
#crbug-todo-migration
Bug: b/321899722
Change-Id: Iee14d10d544e9f0ec046117cc4ec8a55c427adc0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5469947
Reviewed-by: Darryl James <dljames@chromium.org>
Owners-Override: Alison Gale <agale@chromium.org>
Commit-Queue: Alison Gale <agale@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1290838}
The canonical bug format is TODO(crbug.com/<id>). TODOs of the
following forms will all be migrated to the new format:
- TODO(crbug.com/<old id>)
- TODO(https://crbug.com/<old id>)
- TODO(crbug/<old id>)
- TODO(crbug/monorail/<old id>)
- TODO(<old id>)
- TODO(issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/u/1/issues/<old id>)
- TODO(bugs.chromium.org/<old id>)
Bug id mapping is sourced from go/chrome-on-buganizer-prod-issues.
See go/crbug-todo-migration for details.
#crbug-todo-migration
Bug: b/321899722
Change-Id: Iabdfea2fd5393d6bbc54390ca0c995eb2c55bbaa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5469882
Reviewed-by: Darryl James <dljames@chromium.org>
Owners-Override: Alison Gale <agale@chromium.org>
Commit-Queue: Alison Gale <agale@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1290673}
The canonical bug format is TODO(crbug.com/<id>). TODOs of the
following forms will all be migrated to the new format:
- TODO(crbug.com/<old id>)
- TODO(https://crbug.com/<old id>)
- TODO(crbug/<old id>)
- TODO(crbug/monorail/<old id>)
- TODO(<old id>)
- TODO(issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/u/1/issues/<old id>)
- TODO(bugs.chromium.org/<old id>)
Bug id mapping is sourced from go/chrome-on-buganizer-prod-issues.
See go/crbug-todo-migration for details.
#crbug-todo-migration
Bug: b/321899722
Change-Id: Ifd155bbeff882ea939f74cf8b8f847f42847940b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5468156
Reviewed-by: Darryl James <dljames@chromium.org>
Owners-Override: Alison Gale <agale@chromium.org>
Commit-Queue: Alison Gale <agale@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1290297}
The canonical bug format is TODO(crbug.com/<id>). TODOs of the
following forms will all be migrated to the new format:
- TODO(crbug.com/<old id>)
- TODO(https://crbug.com/<old id>)
- TODO(crbug/<old id>)
- TODO(crbug/monorail/<old id>)
- TODO(<old id>)
- TODO(issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/u/1/issues/<old id>)
- TODO(bugs.chromium.org/<old id>)
Bug id mapping is sourced from go/chrome-on-buganizer-prod-issues.
See go/crbug-todo-migration for details.
#crbug-todo-migration
Bug: b/321899722
Change-Id: Ieeb461e2d489e86fd50b87a2a0721a2be34520c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5467317
Owners-Override: Alison Gale <agale@chromium.org>
Commit-Queue: Darryl James <dljames@chromium.org>
Commit-Queue: Alison Gale <agale@chromium.org>
Reviewed-by: Darryl James <dljames@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1290198}
This is a reland of commit a07bd1c23b.
It was reverted because `child_locks_.end()` was being accessed after
`child_locks_` was destroyed.
This reland fixes that and moves the logic to iterate
`pending_callbacks_` of a Pending subtree into a single function.
Patchset 1 is the original reverted commit. Patchset 2 is the fix.
Bug: 332173158
Change-Id: Ibe6613a7a463b636f7f82029491d03a87c0606a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5455255
Reviewed-by: Daseul Lee <dslee@chromium.org>
Commit-Queue: Nathan Memmott <memmott@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1288090}
The Lock Manager had some edge cases where Locks were not being cleaned
up properly. This fixes them and adds tests for each of them.
Bug: 332173158
Change-Id: Ia3bad2ed65254a2a85d55a415e7681d17762a990
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5448348
Reviewed-by: Daseul Lee <dslee@chromium.org>
Commit-Queue: Nathan Memmott <memmott@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1286777}
This reverts commit ecb96d59c4.
Reason for revert: This change is unfortunately causing test flakes again, so need to revert and find the true cause of the flakiness before re-landing again. (crbug.com/332592798)
Original change's description:
> Reland "FilePathWatcher: Synchronously start watching on Mac"
>
> This is a reland of commit b8bf2ad060
>
> The original CL was reverted due to a failing browsertest
> (https://ci.chromium.org/ui/p/chromium/builders/findit/test-single-revision/3436/overview).
>
> It was later determined that this specific test was flaky (https://crbug.com/40939929).
>
> This flake has been resolved as a result of CL: crrev.com/c/5205950
>
> Now that the failing / flaky test has been fixed, this change is ready to re-land in order to unblock FSA Change Observers implementation on Mac.
>
> Original change's description:
> > FilePathWatcher: Synchronously start watching on Mac
> >
> > This CL includes two key changes:
> >
> > 1) Moves all manipulation of the event stream (start, stop, etc)
> > to the sequence the FilePathWatcher lives on
> >
> > Prior to this CL, the event stream was manipulated on the dispatch
> > queue. This does not appear to have been necessary. Moving the stream
> > manipulation to the sequence the FilePathWatcher lives on allows the
> > event stream to be started synchronously, from the Watch() method
> >
> > 2) Use kFSEventStreamEventIdSinceNow
> >
> > Prior to this CL, the event stream was started via async dispatch in
> > the Watch() method. FSEventsGetCurrentEventId() was used to
> > synchronously get the ID of the most recent (system-wide) event, to
> > ensure that any changes between when Watch() returned and when the
> > stream asynchronously started would be played back as historical
> > events. See the documentation of the sinceWhen field here:
> > https://developer.apple.com/documentation/coreservices/1443980-fseventstreamcreate
> >
> > Since the stream is now started synchronously,
> > kFSEventStreamEventIdSinceNow is sufficient for ensuring that all
> > changes after Watch() returns are noticed
> >
> > Note that creating a stream with kFSEventStreamEventIdSinceNow will
> > no longer fire a "history done" event shortly after stream creation.
> > Any callers which relied on the this behavior may break.
> > See the documentation here:
> > https://developer.apple.com/documentation/coreservices/kfseventstreameventflaghistorydone
> >
> > Bug: 1019297
> > Change-Id: Ib720f8b19d0309714d153de974404fab25b16894
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4814677
> > Reviewed-by: Wez <wez@chromium.org>
> > Commit-Queue: Austin Sullivan <asully@chromium.org>
> > Reviewed-by: Robert Sesek <rsesek@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1218981}
>
> Bug: 1019297
> Change-Id: I5d9297c7b3e187bcfdaeaaaf5946984f082b1877
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5274142
> Commit-Queue: Christine Hollingsworth <christinesm@chromium.org>
> Reviewed-by: Daseul Lee <dslee@chromium.org>
> Reviewed-by: Wez <wez@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1281149}
Bug: 1019297
Change-Id: I93b57f4e248fdd79132d6b0e9d92d4aa6b15e1e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5424818
Commit-Queue: Mark Mentovai <mark@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Daseul Lee <dslee@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Auto-Submit: Christine Hollingsworth <christinesm@chromium.org>
Reviewed-by: Christine Hollingsworth <christinesm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1283981}
Our move() implementation always checked the permissions of the parent
directory - creating a synthetic FileSystemHandle in the process - even
for renames where the parent is not relevant. This CL instead checks
permissions of the sibling file without creating a synthetic handle
Since the characteristics of this permission check are still not
specified - which resulted in DCHECK crashes when renaming files in a
BucketFS - this CL also adds a temporary hack to ensure that permission
is always granted for files in a BucketFS. See the comment in the
implementation of GetSharedHandleStateForSandboxedPath() for details
Bug: 40270249, 40198034
Change-Id: I877d12624936fcb46e63924ea5844d5b00db6b25
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4655623
Reviewed-by: Nathan Memmott <memmott@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1282858}
This is a reland of commit b8bf2ad060
The original CL was reverted due to a failing browsertest
(https://ci.chromium.org/ui/p/chromium/builders/findit/test-single-revision/3436/overview).
It was later determined that this specific test was flaky (https://crbug.com/40939929).
This flake has been resolved as a result of CL: crrev.com/c/5205950
Now that the failing / flaky test has been fixed, this change is ready to re-land in order to unblock FSA Change Observers implementation on Mac.
Original change's description:
> FilePathWatcher: Synchronously start watching on Mac
>
> This CL includes two key changes:
>
> 1) Moves all manipulation of the event stream (start, stop, etc)
> to the sequence the FilePathWatcher lives on
>
> Prior to this CL, the event stream was manipulated on the dispatch
> queue. This does not appear to have been necessary. Moving the stream
> manipulation to the sequence the FilePathWatcher lives on allows the
> event stream to be started synchronously, from the Watch() method
>
> 2) Use kFSEventStreamEventIdSinceNow
>
> Prior to this CL, the event stream was started via async dispatch in
> the Watch() method. FSEventsGetCurrentEventId() was used to
> synchronously get the ID of the most recent (system-wide) event, to
> ensure that any changes between when Watch() returned and when the
> stream asynchronously started would be played back as historical
> events. See the documentation of the sinceWhen field here:
> https://developer.apple.com/documentation/coreservices/1443980-fseventstreamcreate
>
> Since the stream is now started synchronously,
> kFSEventStreamEventIdSinceNow is sufficient for ensuring that all
> changes after Watch() returns are noticed
>
> Note that creating a stream with kFSEventStreamEventIdSinceNow will
> no longer fire a "history done" event shortly after stream creation.
> Any callers which relied on the this behavior may break.
> See the documentation here:
> https://developer.apple.com/documentation/coreservices/kfseventstreameventflaghistorydone
>
> Bug: 1019297
> Change-Id: Ib720f8b19d0309714d153de974404fab25b16894
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4814677
> Reviewed-by: Wez <wez@chromium.org>
> Commit-Queue: Austin Sullivan <asully@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1218981}
Bug: 1019297
Change-Id: I5d9297c7b3e187bcfdaeaaaf5946984f082b1877
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5274142
Commit-Queue: Christine Hollingsworth <christinesm@chromium.org>
Reviewed-by: Daseul Lee <dslee@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1281149}
pages"
This is a reland of 65d2c48aaf
Reason for revert: The test was failing on linux-bfcache-rel
Revert CL: https://crrev.com/c/5386731
Original change's description:
> FSA Observer: Discard change notifications to non-fully active pages
>
> This change adds a check for the current
> RenderFrameHost::LifecycleState in the Browser process. File system
> change notifications are now sent to the Renderer only for fully
> active pages.
>
> Bug: 40283779
> Change-Id: I90f6aa52e3a7d1513a286fc61ff0921c948518b5
> Reviewed-on: https://crrev.com/c/5350591
> Reviewed-by: Rahul Singh <rahsin@microsoft.com>
> Commit-Queue: Rahul Singh <rahsin@microsoft.com>
> Reviewed-by: Austin Sullivan <asully@chromium.org>
> Reviewed-by: Christine Hollingsworth <christinesm@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1276479}
Bug: 40283779
Change-Id: I0f47aa417992d9e02708020e5188ad16348c3f14
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5387021
Commit-Queue: Rahul Singh <rahsin@microsoft.com>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Reviewed-by: Christine Hollingsworth <christinesm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1277087}
This change adds a check for the current RenderFrameHost::LifecycleState
in the Browser process. File system change
notifications are now sent to the Renderer only for fully active pages.
Bug: 40283779
Change-Id: I90f6aa52e3a7d1513a286fc61ff0921c948518b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5350591
Reviewed-by: Rahul Singh <rahsin@microsoft.com>
Commit-Queue: Rahul Singh <rahsin@microsoft.com>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Reviewed-by: Christine Hollingsworth <christinesm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1276479}
When a Pending Lock is being promoted to a Taken Lock and none of its
pending_callbacks_ keep their LockHandle alive, the Pending Lock will be
destroyed before the pending_callbacks_ have been cleared.
Fixed: 41494615
Change-Id: I5e9f8c1e5a9a6820dc57178ee00cf5d1d56113dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5313923
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Nathan Memmott <memmott@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1263668}
Currently, taking a lock on a file path fails if the components of the
file path contains any same name amongst themselves. For example,
"/foo/foo" or "/foo/bar/foo" fails. This is due to `child_locks_` map
is keyed on a component name (i.e. "foo", "bar"); instead, it should use
a unique key, such as a full file path.
Bug: 1523851
Change-Id: If1dd17401ababa5a82589eec3170ea1f9bf165c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5262943
Commit-Queue: Daseul Lee <dslee@chromium.org>
Reviewed-by: Christine Hollingsworth <christinesm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1256302}
Updates the timeout value used in Change Observers
browsertests to use `action_timeout()` instead of
`tiny_timeout()` to help resolve test flakiness issue.
Bug: 1499075
Change-Id: I4703d419f3bb1a92ab9f373f7266bd61219cd5c0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5205950
Commit-Queue: Christine Hollingsworth <christinesm@chromium.org>
Reviewed-by: Daseul Lee <dslee@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1249052}
Resolving symbolic links via `base::MakeAbsoluteFilePath()` only
works on POSIX, but not on Windows. Enable the partial fix on
POSIX only for now.
Bug: 1378484
Change-Id: I3a590153c320717f5a57cc777f8f50dec38d3b1b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5165788
Commit-Queue: Daseul Lee <dslee@chromium.org>
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1243601}
In production code, a compromised renderer can call the mojo function
FileSystemAccessAccessHandleHost.Close multiple times on the same
access handle. This puts chrome in an invalid state. This fixes it by
killing the renderer if it attempts to do this.
Fixed: 1514301
Change-Id: I0b148fd908d5caeab223514efe0e9cd5eb0aa933
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5160388
Reviewed-by: Daseul Lee <dslee@chromium.org>
Commit-Queue: Daseul Lee <dslee@chromium.org>
Auto-Submit: Nathan Memmott <memmott@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1242153}
SelectFileDialog::Listener has two variants for each type of
selection. Consolidate each of those two variants into one.
Fixed: 1514382
Low-Coverage-Reason: LARGE_SCALE_REFACTOR This is a refactor of how a specific interface works
Change-Id: I6cf8c2ebb36ba30b351c0108dd7cad848ace9380
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5148579
Commit-Queue: Avi Drissman <avi@chromium.org>
Owners-Override: Avi Drissman <avi@chromium.org>
Reviewed-by: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1241424}
This CL notifies subscribers of file creation events originating from
`window.showSaveFilePicker()`. This will be used by Holding Space to
conditionally add items to the user's Tote when files are created from
certain domains.
Bug: b:310708275
Change-Id: I3f05c6994a10cce136a51e82d22d3572a8393cbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5026500
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1240361}
(1) When checking blocklists, use a resolved path returned from
`base::MakeAbsoluteFilePath()`, which is expected to resolve any
symbolic link.
(2) Additionally, check for blocklist when getting a file handle or
entries (that are files) from a directory handle. With (1), this check
will make sure any new symlink created after the initial check on the
parent directory is caught and re-run with blocklist check on fully
resolved path.
Previously, crrev.com/c/4005144 attempted to handle case (2) partially
on POSIX, using `base::IsLink()` and `base::ReadSymbolicLink()`, causing
some potential bugs. This CL re-attempts to fix the issue using
`base::MakeAbsoluteFilePath()`, which is available on both POSIX and
Windows.
Both features are disabled and will be enabled after testing.
Bug: 1378484
Change-Id: If319359492c08dd829b42262918ad208bbc351c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5068377
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Commit-Queue: Daseul Lee <dslee@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1234657}
SAHs hold locks on their file. When we attempt to create another SAH on
that file, we will evict the current lock if we need to and we can.
Until the lock is destroyed, the new SAH is waiting to be created. The
destruction of the old SAH triggers the destruction of the lock which
then triggers the creation of the new SAH.
In the FileSystemAccessManagerImpl, this means
`access_handle_host_receivers_`'s `erase` could call into its `insert`
which is undefined behavior. This separates the erasing from that set
and the destruction of the SAH.
This bug only happened in incognito because of some checks that are
skipped in incognito which allows all of this to happen synchronously.
Bug: 1382215
Change-Id: Ib92d6de2843710472450def0453d63db6756464c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5081251
Reviewed-by: Daseul Lee <dslee@chromium.org>
Commit-Queue: Nathan Memmott <memmott@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1233015}
Currently the BFCache changes are enabled by the same flag that the
locking scheme feature is. This separates them so that we can kill just
the BFCache changes if needed.
Is enabled by default but the changes are only enabled if both the
BFCache flag and the locking scheme flag are enabled. That way by
default the BFCache changes are controlled by the locking scheme flag.
Bug: 1382215, 1241174
Change-Id: Ibe88c3f87f9564eeb3e80e11c1b00487d306277c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5050409
Reviewed-by: Daseul Lee <dslee@chromium.org>
Commit-Queue: Daseul Lee <dslee@chromium.org>
Auto-Submit: Nathan Memmott <memmott@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1228092}
This is the final changes needed to implement BFCache logic for the File
System Access API. This change makes it so that when we attempt to take
a Lock on file with an existing contentious Lock, we can evict the Lock
if it is held by only inactive pages, and return the new Lock after
eviction.
When we evict a Lock to take a new Lock, the new Lock is pending until
the evicting Lock has been evicted. The pending Lock is not given to the
TakeLock caller until it is no longer pending.
While it is pending though, it is still considered an active Lock, and
all further calls to TakeLock will assume it exists.
If we attempt to take a Lock that is a child of a pending Lock, then it
will also be pending and the caller will not receive the Lock until the
ancestor pending Lock's evicting Lock has been evicted.
If we attempt to take a Lock that is in contention with a pending Lock,
then we can also evict the pending Lock if it is held only by inactive
pages. The callers for the evicting pending Lock will never receive it.
And the new pending Lock waits for the eviction of the original evicting
Lock that the evicting pending Lock was waiting on.
If we evict a Lock that has multiple pending Lock children, then the new
Lock must wait for the eviction of all the child pending Locks' evicting
locks.
Bug: 1382215, 1241174
Change-Id: Iabd602a6ae37bfb69f10e13f08cfb3c1c77bdbd2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4994423
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Nathan Memmott <memmott@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1226460}
This CL includes two key changes:
1) Moves all manipulation of the event stream (start, stop, etc)
to the sequence the FilePathWatcher lives on
Prior to this CL, the event stream was manipulated on the dispatch
queue. This does not appear to have been necessary. Moving the stream
manipulation to the sequence the FilePathWatcher lives on allows the
event stream to be started synchronously, from the Watch() method
2) Use kFSEventStreamEventIdSinceNow
Prior to this CL, the event stream was started via async dispatch in
the Watch() method. FSEventsGetCurrentEventId() was used to
synchronously get the ID of the most recent (system-wide) event, to
ensure that any changes between when Watch() returned and when the
stream asynchronously started would be played back as historical
events. See the documentation of the sinceWhen field here:
https://developer.apple.com/documentation/coreservices/1443980-fseventstreamcreate
Since the stream is now started synchronously,
kFSEventStreamEventIdSinceNow is sufficient for ensuring that all
changes after Watch() returns are noticed
Note that creating a stream with kFSEventStreamEventIdSinceNow will
no longer fire a "history done" event shortly after stream creation.
Any callers which relied on the this behavior may break.
See the documentation here:
https://developer.apple.com/documentation/coreservices/kfseventstreameventflaghistorydone
Bug: 1019297
Change-Id: Ib720f8b19d0309714d153de974404fab25b16894
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4814677
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1218981}
