New issue
Advanced search Search tips

Issue 762642 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 714463
issue 759867



Sign in to add a comment

Support chrome.runtime.reload() in AppShell

Project Member Reported by michae...@chromium.org, Sep 6 2017

Issue description

AppShell should unload and then reload the app when chrome.runtime.reload() is called.

As a bonus, it should avoid quitting when it does so ( issue 759867 ).
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 28 2017

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

commit 023e35323c39bedcadf24600ebe99817beedde3f
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Thu Sep 28 23:21:21 2017

ExtensionRegistrar for enabling and disabling extensions

Classes will use ExtensionRegistrar to enable and disable extensions, as
well as add, remove, reload, and terminate them (later CLs).

Currently Chrome's ExtensionService does this, among many other things.
A standalone class for this is preferred because:
* Code outside //chrome uses similar steps to run extensions, so we
  should share this code in //extensions
* ExtensionService already does too much and is very complex

This CL adds EnableExtension() and DisableExtension() to
ExtensionRegistrar. ExtensionService still does some Chrome-specific
work, but later we'll factor that out into a delegate.

Later CLs will add the other extension lifecycle methods, so eventually
ExtensionRegistrar is the only class allowed to change the
ExtensionRegistry (thus the name). We'll also update calls to
ExtensionService methods to use ExtensionRegistrar directly when it
makes sense.


Design doc: https://goo.gl/trZKep (Google-internal).

Bug:  762642 
Test: extensions_unittests: ExtensionRegistrarTest
Change-Id: I2910a76f873122fbc405b7cc76c766b00a8b216f
Reviewed-on: https://chromium-review.googlesource.com/671206
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505206}
[modify] https://crrev.com/023e35323c39bedcadf24600ebe99817beedde3f/chrome/browser/extensions/extension_service.cc
[modify] https://crrev.com/023e35323c39bedcadf24600ebe99817beedde3f/chrome/browser/extensions/extension_service.h
[modify] https://crrev.com/023e35323c39bedcadf24600ebe99817beedde3f/extensions/browser/BUILD.gn
[add] https://crrev.com/023e35323c39bedcadf24600ebe99817beedde3f/extensions/browser/extension_registrar.cc
[add] https://crrev.com/023e35323c39bedcadf24600ebe99817beedde3f/extensions/browser/extension_registrar.h
[add] https://crrev.com/023e35323c39bedcadf24600ebe99817beedde3f/extensions/browser/extension_registrar_unittest.cc
[modify] https://crrev.com/023e35323c39bedcadf24600ebe99817beedde3f/extensions/browser/extension_system.h
[modify] https://crrev.com/023e35323c39bedcadf24600ebe99817beedde3f/extensions/browser/test_extension_registry_observer.cc
[modify] https://crrev.com/023e35323c39bedcadf24600ebe99817beedde3f/extensions/browser/test_extension_registry_observer.h

Project Member

Comment 2 by bugdroid1@chromium.org, Feb 22 2018

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

commit fd956ce50261cbbf6be7c4e5d1f8e55db213356b
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Thu Feb 22 21:30:58 2018

AppShell: Use ExtensionRegistrar

Moves extension loading code from ShellExtensionSystem to a new
ExtensionLoader class that uses common ExtenstionRegistrar load
functions. ShellExtensionSystem will use ExtensionLoader for
managing extension lifecycles, e.g. reloading.

Bug:  762642 
Change-Id: Ia2ed0a9e58c0d6c2b5cb71f72715f33421deb74f
Reviewed-on: https://chromium-review.googlesource.com/912132
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538570}
[modify] https://crrev.com/fd956ce50261cbbf6be7c4e5d1f8e55db213356b/extensions/shell/BUILD.gn
[add] https://crrev.com/fd956ce50261cbbf6be7c4e5d1f8e55db213356b/extensions/shell/browser/shell_extension_loader.cc
[add] https://crrev.com/fd956ce50261cbbf6be7c4e5d1f8e55db213356b/extensions/shell/browser/shell_extension_loader.h
[add] https://crrev.com/fd956ce50261cbbf6be7c4e5d1f8e55db213356b/extensions/shell/browser/shell_extension_loader_unittest.cc
[modify] https://crrev.com/fd956ce50261cbbf6be7c4e5d1f8e55db213356b/extensions/shell/browser/shell_extension_system.cc
[modify] https://crrev.com/fd956ce50261cbbf6be7c4e5d1f8e55db213356b/extensions/shell/browser/shell_extension_system.h

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 1 2018

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

commit 65478d123ddee27fc9cdcc29621a3e299d2562ee
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Thu Mar 01 07:12:57 2018

AppShell: Support reloading

Implement reloading in ExtensionLoader. Uses keep-alives while apps are
reloading. A future CL will observe these keep-alives to keep app_shell open
during reload.

Bug:  762642 
Change-Id: Ia20b81378d1aeab4ace119d9d2a8b44c2461fdcf
Reviewed-on: https://chromium-review.googlesource.com/912694
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540079}
[modify] https://crrev.com/65478d123ddee27fc9cdcc29621a3e299d2562ee/extensions/browser/extension_registrar.h
[modify] https://crrev.com/65478d123ddee27fc9cdcc29621a3e299d2562ee/extensions/browser/extension_registrar_unittest.cc
[modify] https://crrev.com/65478d123ddee27fc9cdcc29621a3e299d2562ee/extensions/shell/browser/shell_extension_loader.cc
[modify] https://crrev.com/65478d123ddee27fc9cdcc29621a3e299d2562ee/extensions/shell/browser/shell_extension_loader.h
[modify] https://crrev.com/65478d123ddee27fc9cdcc29621a3e299d2562ee/extensions/shell/browser/shell_extension_loader_unittest.cc
[modify] https://crrev.com/65478d123ddee27fc9cdcc29621a3e299d2562ee/extensions/shell/browser/shell_extension_system_factory.cc
[modify] https://crrev.com/65478d123ddee27fc9cdcc29621a3e299d2562ee/extensions/shell/browser/shell_keep_alive_requester.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 1 2018

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

commit a86185d8cfa79bc08c7484b83447de6c172688e6
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Thu Mar 01 08:01:45 2018

Revert "AppShell: Support reloading"

This reverts commit 65478d123ddee27fc9cdcc29621a3e299d2562ee.

Reason for revert: Caused build failure on Mac
https://ci.chromium.org/buildbot/chromium/Mac/38768
https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium%2FMac%2F38768%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

Undefined symbols for architecture x86_64:
  "extensions::ShellNativeAppWindowAura::ShellNativeAppWindowAura(extensions::AppWindow*, extensions::AppWindow::CreateParams const&)", referenced from:
      extensions::(anonymous namespace)::TestAppWindowClient::CreateNativeAppWindow(extensions::AppWindow*, extensions::AppWindow::CreateParams*) in shell_extension_loader_unittest.o
  "extensions::ShellTestBaseAura::SetUp()", referenced from:
      extensions::ShellExtensionLoaderTest::SetUp() in shell_extension_loader_unittest.o
  "extensions::ShellTestBaseAura::ShellTestBaseAura()", referenced from:
      testing::internal::TestFactoryImpl<extensions::ShellExtensionLoaderTest_Extension_Test>::CreateTest() in shell_extension_loader_unittest.o
      testing::internal::TestFactoryImpl<extensions::ShellExtensionLoaderTest_AppLaunch_Test>::CreateTest() in shell_extension_loader_unittest.o
      testing::internal::TestFactoryImpl<extensions::ShellExtensionLoaderTest_AppLaunchAndReload_Test>::CreateTest() in shell_extension_loader_unittest.o
      testing::internal::TestFactoryImpl<extensions::ShellExtensionLoaderTest_NotFound_Test>::CreateTest() in shell_extension_loader_unittest.o
      testing::internal::TestFactoryImpl<extensions::ShellExtensionLoaderTest_ReloadFailure_Test>::CreateTest() in shell_extension_loader_unittest.o
      testing::internal::TestFactoryImpl<extensions::ShellExtensionLoaderTest_LoadAfterReloadFailure_Test>::CreateTest() in shell_extension_loader_unittest.o
      testing::internal::TestFactoryImpl<extensions::ShellExtensionLoaderTest_LoadDisabledExtension_Test>::CreateTest() in shell_extension_loader_unittest.o
      ...
  "extensions::ShellTestBaseAura::~ShellTestBaseAura()", referenced from:
      extensions::ShellExtensionLoaderTest_Extension_Test::~ShellExtensionLoaderTest_Extension_Test() in shell_extension_loader_unittest.o
      extensions::ShellExtensionLoaderTest_Extension_Test::~ShellExtensionLoaderTest_Extension_Test() in shell_extension_loader_unittest.o
      extensions::ShellExtensionLoaderTest_AppLaunch_Test::~ShellExtensionLoaderTest_AppLaunch_Test() in shell_extension_loader_unittest.o
      extensions::ShellExtensionLoaderTest_AppLaunch_Test::~ShellExtensionLoaderTest_AppLaunch_Test() in shell_extension_loader_unittest.o
      extensions::ShellExtensionLoaderTest_AppLaunchAndReload_Test::~ShellExtensionLoaderTest_AppLaunchAndReload_Test() in shell_extension_loader_unittest.o
      extensions::ShellExtensionLoaderTest_AppLaunchAndReload_Test::~ShellExtensionLoaderTest_AppLaunchAndReload_Test() in shell_extension_loader_unittest.o
      extensions::ShellExtensionLoaderTest_NotFound_Test::~ShellExtensionLoaderTest_NotFound_Test() in shell_extension_loader_unittest.o
      ...
  "extensions::ShellTestBaseAura::TearDown()", referenced from:
      extensions::ShellExtensionLoaderTest::TearDown() in shell_extension_loader_unittest.o
  "extensions::ShellTestBaseAura::InitAppWindow(extensions::AppWindow*, gfx::Rect const&)", referenced from:
      extensions::ShellExtensionLoaderTest_AppLaunch_Test::TestBody() in shell_extension_loader_unittest.o
      extensions::ShellExtensionLoaderTest_AppLaunchAndReload_Test::TestBody() in shell_extension_loader_unittest.o
      extensions::ShellExtensionLoaderTest_ReloadFailure_Test::TestBody() in shell_extension_loader_unittest.o
ld: symbol(s) not found for architecture x86_64


Original change's description:
> AppShell: Support reloading
> 
> Implement reloading in ExtensionLoader. Uses keep-alives while apps are
> reloading. A future CL will observe these keep-alives to keep app_shell open
> during reload.
> 
> Bug:  762642 
> Change-Id: Ia20b81378d1aeab4ace119d9d2a8b44c2461fdcf
> Reviewed-on: https://chromium-review.googlesource.com/912694
> Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
> Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#540079}

TBR=michaelpg@chromium.org,rdevlin.cronin@chromium.org

Change-Id: I2db08946671be6eb37fd3f4d658573b87dd3c1b1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  762642 
Reviewed-on: https://chromium-review.googlesource.com/942701
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540098}
[modify] https://crrev.com/a86185d8cfa79bc08c7484b83447de6c172688e6/extensions/browser/extension_registrar.h
[modify] https://crrev.com/a86185d8cfa79bc08c7484b83447de6c172688e6/extensions/browser/extension_registrar_unittest.cc
[modify] https://crrev.com/a86185d8cfa79bc08c7484b83447de6c172688e6/extensions/shell/browser/shell_extension_loader.cc
[modify] https://crrev.com/a86185d8cfa79bc08c7484b83447de6c172688e6/extensions/shell/browser/shell_extension_loader.h
[modify] https://crrev.com/a86185d8cfa79bc08c7484b83447de6c172688e6/extensions/shell/browser/shell_extension_loader_unittest.cc
[modify] https://crrev.com/a86185d8cfa79bc08c7484b83447de6c172688e6/extensions/shell/browser/shell_extension_system_factory.cc
[modify] https://crrev.com/a86185d8cfa79bc08c7484b83447de6c172688e6/extensions/shell/browser/shell_keep_alive_requester.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 9 2018

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

commit aadafe65dedbb2e9644477fcff734707bd93e3a3
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Fri Mar 09 19:27:29 2018

Reland "AppShell: Support reloading"

This is a reland of 65478d123ddee27fc9cdcc29621a3e299d2562ee with
Aura-specific tests moved to a subclass and #ifdef.

Original change's description:
> AppShell: Support reloading
>
> Implement reloading in ExtensionLoader. Uses keep-alives while apps are
> reloading. A future CL will observe these keep-alives to keep app_shell open
> during reload.
>
> Bug:  762642 
> Change-Id: Ia20b81378d1aeab4ace119d9d2a8b44c2461fdcf
> Reviewed-on: https://chromium-review.googlesource.com/912694
> Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
> Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#540079}

Bug:  762642 
Change-Id: I6c516c4736ca2327965e034d10717341e23303dc
Reviewed-on: https://chromium-review.googlesource.com/950342
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542195}
[modify] https://crrev.com/aadafe65dedbb2e9644477fcff734707bd93e3a3/extensions/browser/extension_registrar.h
[modify] https://crrev.com/aadafe65dedbb2e9644477fcff734707bd93e3a3/extensions/browser/extension_registrar_unittest.cc
[modify] https://crrev.com/aadafe65dedbb2e9644477fcff734707bd93e3a3/extensions/shell/browser/shell_extension_loader.cc
[modify] https://crrev.com/aadafe65dedbb2e9644477fcff734707bd93e3a3/extensions/shell/browser/shell_extension_loader.h
[modify] https://crrev.com/aadafe65dedbb2e9644477fcff734707bd93e3a3/extensions/shell/browser/shell_extension_loader_unittest.cc
[modify] https://crrev.com/aadafe65dedbb2e9644477fcff734707bd93e3a3/extensions/shell/browser/shell_extension_system_factory.cc
[modify] https://crrev.com/aadafe65dedbb2e9644477fcff734707bd93e3a3/extensions/shell/browser/shell_keep_alive_requester.cc
[modify] https://crrev.com/aadafe65dedbb2e9644477fcff734707bd93e3a3/extensions/shell/test/shell_test_base_aura.cc
[modify] https://crrev.com/aadafe65dedbb2e9644477fcff734707bd93e3a3/extensions/shell/test/shell_test_helper_aura.cc
[modify] https://crrev.com/aadafe65dedbb2e9644477fcff734707bd93e3a3/extensions/shell/test/shell_test_helper_aura.h

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 9 2018

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

commit c176d79e115597bf129d60eab3a0a74024fb1da8
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Fri Mar 09 22:43:30 2018

AppShell: Enable chrome.runtime.reload()

Allow apps to reload themselves, and ensure the ShellDesktopControllerAura
keeps app_shell alive long enough for the app to come back up.

Add browser tests for ShellDesktopControllerAura and
chrome.runtime.reload() in AppShell.

Bug:  762642 , 759867 
Change-Id: If09ad83e6d3073461ebf8b53afa69a6c2aab7e15
Reviewed-on: https://chromium-review.googlesource.com/912752
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542261}
[modify] https://crrev.com/c176d79e115597bf129d60eab3a0a74024fb1da8/extensions/shell/BUILD.gn
[add] https://crrev.com/c176d79e115597bf129d60eab3a0a74024fb1da8/extensions/shell/browser/api/runtime/runtime_apitest.cc
[rename] https://crrev.com/c176d79e115597bf129d60eab3a0a74024fb1da8/extensions/shell/browser/api/runtime/shell_runtime_api_delegate.cc
[rename] https://crrev.com/c176d79e115597bf129d60eab3a0a74024fb1da8/extensions/shell/browser/api/runtime/shell_runtime_api_delegate.h
[modify] https://crrev.com/c176d79e115597bf129d60eab3a0a74024fb1da8/extensions/shell/browser/shell_browsertest.cc
[modify] https://crrev.com/c176d79e115597bf129d60eab3a0a74024fb1da8/extensions/shell/browser/shell_desktop_controller_aura.cc
[modify] https://crrev.com/c176d79e115597bf129d60eab3a0a74024fb1da8/extensions/shell/browser/shell_desktop_controller_aura.h
[add] https://crrev.com/c176d79e115597bf129d60eab3a0a74024fb1da8/extensions/shell/browser/shell_desktop_controller_aura_browsertest.cc
[modify] https://crrev.com/c176d79e115597bf129d60eab3a0a74024fb1da8/extensions/shell/browser/shell_extension_system.cc
[modify] https://crrev.com/c176d79e115597bf129d60eab3a0a74024fb1da8/extensions/shell/browser/shell_extension_system.h
[modify] https://crrev.com/c176d79e115597bf129d60eab3a0a74024fb1da8/extensions/shell/browser/shell_extensions_browser_client.cc

Status: Fixed (was: Started)

Sign in to add a comment