Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
437603002:
Remove deprecated extension notification from WebstoreStartupInstallerTest (Closed)
|
Created:
6 years, 4 months ago by limasdf Modified:
6 years, 4 months ago Reviewers:
not at google - send to devlin CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Project:
chromium Visibility:
Public. |
DescriptionRemove deprecated extension notification from WebstoreStartupInstallerTest
BUG=354046
TEST=browser_tests WebstoreStartupInstallerTest.*
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287154
Patch Set 1 #
Total comments: 4
Patch Set 2 : #Patch Set 3 : testingprofile -> profile() #Messages
Total messages: 15 (0 generated)
when you have time
lgtm https://codereview.chromium.org/437603002/diff/1/chrome/browser/extensions/we... File chrome/browser/extensions/webstore_startup_installer_browsertest.cc (right): https://codereview.chromium.org/437603002/diff/1/chrome/browser/extensions/we... chrome/browser/extensions/webstore_startup_installer_browsertest.cc:270: << "Unexpected notification type : " << type; eh who cares about the message https://codereview.chromium.org/437603002/diff/1/chrome/browser/extensions/we... chrome/browser/extensions/webstore_startup_installer_browsertest.cc:271: browser_open_count_++; ++brower_open_count_;
https://codereview.chromium.org/437603002/diff/1/chrome/browser/extensions/we... File chrome/browser/extensions/webstore_startup_installer_browsertest.cc (right): https://codereview.chromium.org/437603002/diff/1/chrome/browser/extensions/we... chrome/browser/extensions/webstore_startup_installer_browsertest.cc:270: << "Unexpected notification type : " << type; On 2014/07/31 17:24:58, kalman wrote: > eh who cares about the message Done. Removed printing message. https://codereview.chromium.org/437603002/diff/1/chrome/browser/extensions/we... chrome/browser/extensions/webstore_startup_installer_browsertest.cc:271: browser_open_count_++; On 2014/07/31 17:24:58, kalman wrote: > ++brower_open_count_; Done.
The CQ bit was checked by limasdf@gmail.com
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/limasdf@gmail.com/437603002/20001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_chromium_chromeos_rel on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_swarming on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_rel on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...) win8_chromium_rel on tryserver.chromium.win (http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...) win_chromium_rel on tryserver.chromium.win (http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...) win_chromium_x64_rel on tryserver.chromium.win (http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was unchecked by limasdf@gmail.com
The CQ bit was checked by limasdf@gmail.com
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/limasdf@gmail.com/437603002/40001
The CQ bit was unchecked by limasdf@gmail.com
The CQ bit was checked by limasdf@gmail.com
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/limasdf@gmail.com/437603002/40001
Message was sent while issue was closed.
Change committed as 287154
Message was sent while issue was closed.
A revert of this CL has been created in https://codereview.chromium.org/428123004/ by pneubeck@chromium.org. The reason for reverting is: Broke several tests on Linux Asan/Lsan: http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Te... http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Te... http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Te... E.g. ==21932==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000210570 at pc 0x00000c56a19d bp 0x7fff04951d00 sp 0x7fff04951cf8 READ of size 8 at 0x611000210570 thread T0 (browser_tests) #0 0xc56a19c in begin third_party/libc++/trunk/include/vector:1417:12 #1 0xc56a19c in RemoveObserver base/observer_list.h:168 #2 0xc56a19c in extensions::ExtensionRegistry::RemoveObserver(extensions::ExtensionRegistryObserver*) extensions/browser/extension_registry.cc:37 #3 0x13a2de2 in RemoveAll base/scoped_observer.h:39:7 #4 0x13a2de2 in ~ScopedObserver base/scoped_observer.h:22 #5 0x13a2de2 in CommandLineWebstoreInstall::~CommandLineWebstoreInstall() chrome/browser/extensions/webstore_startup_installer_browsertest.cc:250.
Message was sent while issue was closed.
On 2014/08/02 07:06:29, pneubeck wrote: > A revert of this CL has been created in > https://codereview.chromium.org/428123004/ by mailto:pneubeck@chromium.org. > > The reason for reverting is: Broke several tests on Linux Asan/Lsan: > > http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Te... > > http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Te... > > http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Te... > > E.g. > > ==21932==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000210570 > at pc 0x00000c56a19d bp 0x7fff04951d00 sp 0x7fff04951cf8 > READ of size 8 at 0x611000210570 thread T0 (browser_tests) > #0 0xc56a19c in begin third_party/libc++/trunk/include/vector:1417:12 > #1 0xc56a19c in RemoveObserver base/observer_list.h:168 > #2 0xc56a19c in > extensions::ExtensionRegistry::RemoveObserver(extensions::ExtensionRegistryObserver*) > extensions/browser/extension_registry.cc:37 > #3 0x13a2de2 in RemoveAll base/scoped_observer.h:39:7 > #4 0x13a2de2 in ~ScopedObserver base/scoped_observer.h:22 > #5 0x13a2de2 in CommandLineWebstoreInstall::~CommandLineWebstoreInstall() > chrome/browser/extensions/webstore_startup_installer_browsertest.cc:250. The ExtensionRegistryObserver probably needs to be removed earlier in the test teardown, perhaps in TearDownOnMainThread. |
