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

Issue 800641 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Last visit > 30 days ago
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Revoke mojo interfaces on ExecutionContext destruction

Project Member Reported by austi...@google.com, Jan 10 2018

Issue description

Mojo interfaces used by web APIs are currently not tied to an execution context, meaning that on a cross-origin navigation, data could be leaked from the previous origin if these handles are not explicitly closed. We should automatically close these handles on context destruction by tying the lifetimes of mojo interfaces to an execution context.

Design doc: https://docs.google.com/document/d/15i6XSTSUL_VbDn5UEcy1NLOO1P2OpYqKHFNlng04OwM/edit?usp=sharing
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 25 2018

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

commit 88931be92521d09ad69338934561054259b1f2de
Author: Austin Tankiang <austinct@google.com>
Date: Thu Jan 25 05:54:57 2018

Add InterfaceInvalidator and WeakInterfacePtr classes

Mojo InterfacePtrs need a way to be automatically revoked, so wrap them
in a WeakInterfacePtr class. This class is associated with an
InterfaceInvalidator class that revokes the weak interface pointer when
the invalidator is destroyed.

Add tests to verify that invalidation works well with the existing
InterfacePtr functions. Change PingService to be [Sync] so both async
and sync calls can be tested.

Also introduce a new typedef for blink code, |WeakFooPtr|. Existing code
will be gradually changed to use the new typedef.

Bug: 800641
Change-Id: I29778b233b81eec14ffca048a70f84cc3e31fb44
Reviewed-on: https://chromium-review.googlesource.com/848432
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Austin Tankiang <austinct@google.com>
Cr-Commit-Position: refs/heads/master@{#531823}
[modify] https://crrev.com/88931be92521d09ad69338934561054259b1f2de/mojo/public/cpp/bindings/lib/interface_ptr_state.h
[modify] https://crrev.com/88931be92521d09ad69338934561054259b1f2de/mojo/public/interfaces/bindings/tests/ping_service.mojom
[modify] https://crrev.com/88931be92521d09ad69338934561054259b1f2de/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl
[modify] https://crrev.com/88931be92521d09ad69338934561054259b1f2de/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/88931be92521d09ad69338934561054259b1f2de/third_party/WebKit/Source/platform/mojo/DEPS
[add] https://crrev.com/88931be92521d09ad69338934561054259b1f2de/third_party/WebKit/Source/platform/mojo/InterfaceInvalidator.cpp
[add] https://crrev.com/88931be92521d09ad69338934561054259b1f2de/third_party/WebKit/Source/platform/mojo/InterfaceInvalidator.h
[add] https://crrev.com/88931be92521d09ad69338934561054259b1f2de/third_party/WebKit/Source/platform/mojo/InterfaceInvalidatorTest.cpp
[add] https://crrev.com/88931be92521d09ad69338934561054259b1f2de/third_party/WebKit/Source/platform/mojo/WeakInterfacePtr.h

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 29 2018

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

commit 762a427c366d407bce92f9a8c1e5b988be4770ff
Author: Austin Tankiang <austinct@google.com>
Date: Mon Jan 29 03:01:26 2018

Add an InterfaceInvalidator to ExecutionContexts

This allows the lifetime of WeakInterfacePtrs to be tied to an
ExecutionContext, as now the interface can be tied to an invalidator whose
lifetime is bound to an ExecutionContext.

Bug: 800641
Change-Id: I0c922c90a89108f56c179b74170a8e0e2a5f666f
Reviewed-on: https://chromium-review.googlesource.com/872470
Commit-Queue: Austin Tankiang <austinct@google.com>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532310}
[modify] https://crrev.com/762a427c366d407bce92f9a8c1e5b988be4770ff/third_party/WebKit/Source/core/dom/DocumentTest.cpp
[modify] https://crrev.com/762a427c366d407bce92f9a8c1e5b988be4770ff/third_party/WebKit/Source/core/dom/ExecutionContext.cpp
[modify] https://crrev.com/762a427c366d407bce92f9a8c1e5b988be4770ff/third_party/WebKit/Source/core/dom/ExecutionContext.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 30 2018

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

commit 3758559165c188d04c3c9542e50ed240a0d8d14a
Author: Austin Tankiang <austinct@google.com>
Date: Tue Jan 30 06:30:08 2018

Use WeakInterfacePtr for Geolocation

Make Geolocation use WeakInterfacePtr, which is the class to use
so that mojo interface handles do not need to be manually closed
on Execution context destruction.

WeakInterfacePtr are the new hotness, they do that automatically
on Execution context destruction.

No new tests, no change in behavior, covered by existing tests.

Bug: 800641
Change-Id: Icf5347c012c735b03676d2537cae4473f03fc568
Reviewed-on: https://chromium-review.googlesource.com/880246
Commit-Queue: Austin Tankiang <austinct@google.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532795}
[modify] https://crrev.com/3758559165c188d04c3c9542e50ed240a0d8d14a/third_party/WebKit/Source/modules/geolocation/Geolocation.cpp
[modify] https://crrev.com/3758559165c188d04c3c9542e50ed240a0d8d14a/third_party/WebKit/Source/modules/geolocation/Geolocation.h

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 5 2018

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

commit 3ef3ba3c021ad2f4ffd32d33e1bedbf1406f7a13
Author: Austin Tankiang <austinct@google.com>
Date: Mon Feb 05 03:06:55 2018

Allow InterfaceProvider::GetInterface() to take in weak interfaces

Make InterfaceProvider::GetInterface() generically forward arguments to
MakeRequest. This is needed as service manager cannot depend on the weak
interfaces in blink, so use ADL to find the correct MakeRequest instead.

Rename the GetInterface() overload that takes in a string to be
GetInterfaceByName(), as it gets shadowed by the templated overload.

Change Geolocation to use the new GetInterface().

No new behavior, changes covered by existing tests.

Bug: 800641
Change-Id: Ie4d270ed7e9091b7f5f4e82bec429b2cc54f3f37
Reviewed-on: https://chromium-review.googlesource.com/897064
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Austin Tankiang <austinct@google.com>
Cr-Commit-Position: refs/heads/master@{#534316}
[modify] https://crrev.com/3ef3ba3c021ad2f4ffd32d33e1bedbf1406f7a13/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/3ef3ba3c021ad2f4ffd32d33e1bedbf1406f7a13/services/service_manager/public/cpp/interface_provider.cc
[modify] https://crrev.com/3ef3ba3c021ad2f4ffd32d33e1bedbf1406f7a13/services/service_manager/public/cpp/interface_provider.h
[modify] https://crrev.com/3ef3ba3c021ad2f4ffd32d33e1bedbf1406f7a13/third_party/WebKit/Source/core/mojo/Mojo.cpp
[modify] https://crrev.com/3ef3ba3c021ad2f4ffd32d33e1bedbf1406f7a13/third_party/WebKit/Source/modules/geolocation/Geolocation.cpp

Description: Show this description
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 8 2018

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

commit 72a329b7334de7a3a2ddaa8d30d4d4e9b4a2cf0f
Author: Austin Tankiang <austinct@google.com>
Date: Thu Feb 08 00:35:06 2018

Add revocable binding and revocable strong binding classes

RevocableBinding and RevocableStrongBinding are the binding/strong
binding counterpart for RevocableInterfacePtr, which was previously
called WeakInterfacePtr (see crrev.com/c/848432). Change the prefix from
"Weak" to "Revocable" as WeakStrongBinding is not a good name. These
classes are used to tie the lifetime of bindings in blink to another
object, e.g. ExecutionContexts.

Add tests to verify that invalidation works in conjunction with
existing bindings functionality.

Bug: 800641
Change-Id: I781796d2fc154ccef7feeaf203ffc7c3a3cebe0f
Reviewed-on: https://chromium-review.googlesource.com/892839
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Austin Tankiang <austinct@google.com>
Cr-Commit-Position: refs/heads/master@{#535215}
[modify] https://crrev.com/72a329b7334de7a3a2ddaa8d30d4d4e9b4a2cf0f/mojo/public/cpp/bindings/binding.h
[modify] https://crrev.com/72a329b7334de7a3a2ddaa8d30d4d4e9b4a2cf0f/mojo/public/cpp/bindings/lib/binding_state.h
[modify] https://crrev.com/72a329b7334de7a3a2ddaa8d30d4d4e9b4a2cf0f/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl
[modify] https://crrev.com/72a329b7334de7a3a2ddaa8d30d4d4e9b4a2cf0f/third_party/WebKit/Source/modules/geolocation/Geolocation.h
[modify] https://crrev.com/72a329b7334de7a3a2ddaa8d30d4d4e9b4a2cf0f/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/72a329b7334de7a3a2ddaa8d30d4d4e9b4a2cf0f/third_party/WebKit/Source/platform/mojo/InterfaceInvalidatorTest.cpp
[add] https://crrev.com/72a329b7334de7a3a2ddaa8d30d4d4e9b4a2cf0f/third_party/WebKit/Source/platform/mojo/RevocableBinding.h
[rename] https://crrev.com/72a329b7334de7a3a2ddaa8d30d4d4e9b4a2cf0f/third_party/WebKit/Source/platform/mojo/RevocableInterfacePtr.h
[add] https://crrev.com/72a329b7334de7a3a2ddaa8d30d4d4e9b4a2cf0f/third_party/WebKit/Source/platform/mojo/RevocableStrongBinding.h

Cc: dcheng@chromium.org

Sign in to add a comment