New issue
Advanced search Search tips

Issue 886957 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 26
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

COM related code fix/cleanup for Win10 native notification

Project Member Reported by chengx@chromium.org, Sep 19

Issue description

OS: Win 10

Let's use this bug to keep track the changes. 

 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 19

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

commit 357386d1517b830493b694870e3adf23ea071a61
Author: Xi Cheng <chengx@chromium.org>
Date: Wed Sep 19 17:09:30 2018

Make MockIToastNotifier derive from RuntimeClass to simply implementation

Bug:  886957 
Change-Id: I53e8736e548c96ff623b4543376bface5d6eb013
Reviewed-on: https://chromium-review.googlesource.com/1232526
Commit-Queue: Xi Cheng <chengx@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592448}
[modify] https://crrev.com/357386d1517b830493b694870e3adf23ea071a61/chrome/browser/notifications/win/mock_itoastnotifier.cc
[modify] https://crrev.com/357386d1517b830493b694870e3adf23ea071a61/chrome/browser/notifications/win/mock_itoastnotifier.h

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 19

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

commit c2fbd13843b8c61c29824e260f610fa5f4254ee9
Author: Xi Cheng <chengx@chromium.org>
Date: Wed Sep 19 18:32:44 2018

Reland "Fix a few memory leaks in NotificationPlatformBridgeWin"

This is a reland of 3d28a29b775e4d0828b4b50267ce090a8b0793a9.

In the original change, NotificationPlatformBridgeWinTest.Suppress and
NotificationPlatformBridgeWinUITest.GetDisplayed failed in win-asan bot.

This CL fixed it.

The fix also required MockIToastNotification to derive from RuntimeClass,
which significantly simplified its implementation.

Original change's description:
> Fix a few memory leaks in NotificationPlatformBridgeWin
>
> We should 1) wrap the bare pointers in ComPtr; and 2) attach
> QueryInterfaces to ComPtr directly.
>
> Bug:  884224 
> Change-Id: I91cb5c7312d8e24f59962ffb33faadaba36bd7ef
> Reviewed-on: https://chromium-review.googlesource.com/1227493
> Commit-Queue: Xi Cheng <chengx@chromium.org>
> Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
> Reviewed-by: Robert Liao <robliao@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#591737}

Bug:  884224 ,  886957 
Change-Id: If2674299437f04f637f6b2b8278b40a4dbf7b9d0
Reviewed-on: https://chromium-review.googlesource.com/1228991
Commit-Queue: Xi Cheng <chengx@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592487}
[modify] https://crrev.com/c2fbd13843b8c61c29824e260f610fa5f4254ee9/chrome/browser/notifications/notification_platform_bridge_win.cc
[modify] https://crrev.com/c2fbd13843b8c61c29824e260f610fa5f4254ee9/chrome/browser/notifications/notification_platform_bridge_win.h
[modify] https://crrev.com/c2fbd13843b8c61c29824e260f610fa5f4254ee9/chrome/browser/notifications/notification_platform_bridge_win_interactive_uitest.cc
[modify] https://crrev.com/c2fbd13843b8c61c29824e260f610fa5f4254ee9/chrome/browser/notifications/notification_platform_bridge_win_unittest.cc
[modify] https://crrev.com/c2fbd13843b8c61c29824e260f610fa5f4254ee9/chrome/browser/notifications/win/mock_itoastnotification.cc
[modify] https://crrev.com/c2fbd13843b8c61c29824e260f610fa5f4254ee9/chrome/browser/notifications/win/mock_itoastnotification.h

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 20

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

commit c0cd03883df4bff8c02aaccaf4b3abf915ccf1c0
Author: Xi Cheng <chengx@chromium.org>
Date: Thu Sep 20 11:40:38 2018

Return empty vector explicitly in GetDisplayedFromActionCenter()

Bug:  886957 
Change-Id: I734a8858b2538aaf6b146ce6c209ab4718ff278c
Reviewed-on: https://chromium-review.googlesource.com/1235132
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Commit-Queue: Xi Cheng <chengx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592757}
[modify] https://crrev.com/c0cd03883df4bff8c02aaccaf4b3abf915ccf1c0/chrome/browser/notifications/notification_platform_bridge_win.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 20

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

commit 6f4a6f4b6bd355386f91c46dc5f384c5ca5a7cd2
Author: Xi Cheng <chengx@chromium.org>
Date: Thu Sep 20 11:41:10 2018

Make MockIToastActivatedEventArgs derive from RuntimeClass

This CL also cleans up the headers.

Bug:  886957 
Change-Id: I0d63484a7b2e96be64bc9e83f9350ca4ad7ffe41
Reviewed-on: https://chromium-review.googlesource.com/1234266
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Commit-Queue: Xi Cheng <chengx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592758}
[modify] https://crrev.com/6f4a6f4b6bd355386f91c46dc5f384c5ca5a7cd2/chrome/browser/notifications/notification_platform_bridge_win_interactive_uitest.cc
[modify] https://crrev.com/6f4a6f4b6bd355386f91c46dc5f384c5ca5a7cd2/chrome/browser/notifications/notification_platform_bridge_win_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 22

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 24

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

commit 6254e556e9f78d8425ce5313b3f7e3a9f1f0acbe
Author: Xi Cheng <chengx@chromium.org>
Date: Mon Sep 24 16:16:31 2018

Simplify namespace for ComPtr

Bug:  886957 
Change-Id: I36adf26e497b0dee187ed151a09f16b966e277a8
Reviewed-on: https://chromium-review.googlesource.com/1240175
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Commit-Queue: Xi Cheng <chengx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593555}
[modify] https://crrev.com/6254e556e9f78d8425ce5313b3f7e3a9f1f0acbe/chrome/browser/notifications/win/mock_itoastnotification.cc
[modify] https://crrev.com/6254e556e9f78d8425ce5313b3f7e3a9f1f0acbe/chrome/notification_helper/com_server_module.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 25

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

commit cf0d87b6c33251c29c23270fed550d04a1b07ffd
Author: Xi Cheng <chengx@chromium.org>
Date: Tue Sep 25 17:50:56 2018

Return ComPtr via move constructor rather than "Double Pointer"

While "Double Pointer" is commonly used in C, let's use (implicit) move
semantics in the modern C++ era.

Bug:  886957 
Change-Id: I15693413a8a2474fb8200fc1778d4f9fe4656905
Reviewed-on: https://chromium-review.googlesource.com/1242298
Commit-Queue: Xi Cheng <chengx@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593996}
[modify] https://crrev.com/cf0d87b6c33251c29c23270fed550d04a1b07ffd/chrome/browser/notifications/notification_platform_bridge_win.cc
[modify] https://crrev.com/cf0d87b6c33251c29c23270fed550d04a1b07ffd/chrome/browser/notifications/notification_platform_bridge_win.h
[modify] https://crrev.com/cf0d87b6c33251c29c23270fed550d04a1b07ffd/chrome/browser/notifications/notification_platform_bridge_win_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment