[Extensions] Update extensions UI to observe events and add test

[Extensions] Update extensions UI to observe events and add test

Instead of completely regenerating all of the data each time anything
extension-y happens, have the extensions ui observe specific events and only
regenerate/update the extensions that were modified. Start by limiting events
to loaded/unloaded and error added events.

Also add a basic browsertest for some of the extensions page functionality.

BUG=461039
Committed: https://crrev.com/4ed20b10757af80bd743ede1742421c312503d14
Cr-Commit-Position: refs/heads/master@{#324140}

[Devlin, 2015-03-30 23:31:17]

Patchset #1 (id:1) has been deleted

[Devlin, 2015-03-31 16:27:21]

rdevlin.cronin@chromium.org changed reviewers:
+ dbeam@chromium.org

[Devlin, 2015-03-31 16:27:22]

Dan, can you take a look at the webui and the resources?  The closure
compiler/idl change is actually a subset of the one in
https://codereview.chromium.org/1016413004/, because for both we want to include
the changed item details in the event firing.

[Dan Beam, 2015-04-01 23:12:04]

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:215:
this.extensionsUpdated = null;
@private?

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:236:
this.removeChild(childNode);
nit: childNode.parentNode.removeChild(childNode);

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/extensions/extension_settings_browsertest.cc
(right):

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.cc:118:
extension = observer.WaitForExtensionLoaded();
can this return nullptr?

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/extensions/extension_settings_browsertest.js
(right):

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:70: //
paths as possible.
this generally sounds like a recipe for flakiness.

you should break this up into like 3 different tests

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:78: 
/** @protected {Array<!Function>} */

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:80: 
all these should be @protected

[Devlin, 2015-04-02 00:01:39]

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:215:
this.extensionsUpdated = null;
On 2015/04/01 23:12:04, Dan Beam wrote:
> @private?

Used by the test - okay to make it private?  Add a getter?  Leave it as is?

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:236:
this.removeChild(childNode);
On 2015/04/01 23:12:04, Dan Beam wrote:
> nit: childNode.parentNode.removeChild(childNode);

Done.

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/extensions/extension_settings_browsertest.cc
(right):

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.cc:118:
extension = observer.WaitForExtensionLoaded();
On 2015/04/01 23:12:04, Dan Beam wrote:
> can this return nullptr?

Nope.

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/extensions/extension_settings_browsertest.js
(right):

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:70: //
paths as possible.
On 2015/04/01 23:12:04, Dan Beam wrote:
> this generally sounds like a recipe for flakiness.
> 
> you should break this up into like 3 different tests

Fair enough.  I try to avoid it when possible, because browser tests are so
expensive, but it probably is too much here.  You okay with leaving all the
functionality in this class?  I personally find it a bit more readable, but
that's just me.

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:78: 
On 2015/04/01 23:12:04, Dan Beam wrote:
> /** @protected {Array<!Function>} */

Done.

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:80: 
On 2015/04/01 23:12:04, Dan Beam wrote:
> all these should be @protected

Done.

[Dan Beam, 2015-04-02 01:34:54]

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:215:
this.extensionsUpdated = null;
On 2015/04/02 00:01:38, Devlin wrote:
> On 2015/04/01 23:12:04, Dan Beam wrote:
> > @private?
> 
> Used by the test - okay to make it private?  Add a getter?  Leave it as is?

yeah, ok to make private, IMO

https://codereview.chromium.org/1049483003/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/extensions/extension_settings_browsertest.js
(right):

https://codereview.chromium.org/1049483003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:74: GEN(' 
InstallPackagedApp();');
why do we need to install all types for this test?

https://codereview.chromium.org/1049483003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:119:
assertTrue($(GOOD_CRX_ID) == null);
assertEQ(null, $(GOOD_CRX_ID))

or

assertFalse($(GOOD_CRX_ID))

https://codereview.chromium.org/1049483003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:135:
chrome.management.setEnabled(GOOD_CRX_ID, false, function() {
, this.nextStep.bind(this));

https://codereview.chromium.org/1049483003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:140:
disable,
can't this just be this.verifyDisableWorks in this case?  seems pretty similar.

i didn't know you have to disable to test that enable works.

[Devlin, 2015-04-02 21:49:37]

Patchset #3 (id:60001) has been deleted

[Devlin, 2015-04-02 21:50:05]

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/1049483003/diff/20001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:215:
this.extensionsUpdated = null;
On 2015/04/02 01:34:53, Dan Beam wrote:
> On 2015/04/02 00:01:38, Devlin wrote:
> > On 2015/04/01 23:12:04, Dan Beam wrote:
> > > @private?
> > 
> > Used by the test - okay to make it private?  Add a getter?  Leave it as is?
> 
> yeah, ok to make private, IMO

Done.

https://codereview.chromium.org/1049483003/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/extensions/extension_settings_browsertest.js
(right):

https://codereview.chromium.org/1049483003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:74: GEN(' 
InstallPackagedApp();');
On 2015/04/02 01:34:54, Dan Beam wrote:
> why do we need to install all types for this test?

It's not so much that we need all types for any of these specific tests so much
as we've had some bugs where we encounter errors just because we never test for
having an extension type x loaded (e.g., never having a shared module).  I
figured that it wasn't really worth having a dedicated test to
"LoadABunchOfExtensionTypes" and do nothing when we could just do that as part
of these tests, but I don't feel strongly about how it's organized (just as long
as we *do* have at least one test that loads these extension types).

https://codereview.chromium.org/1049483003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:119:
assertTrue($(GOOD_CRX_ID) == null);
On 2015/04/02 01:34:54, Dan Beam wrote:
> assertEQ(null, $(GOOD_CRX_ID))
> 
> or
> 
> assertFalse($(GOOD_CRX_ID))

Done.

https://codereview.chromium.org/1049483003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:135:
chrome.management.setEnabled(GOOD_CRX_ID, false, function() {
On 2015/04/02 01:34:54, Dan Beam wrote:
> , this.nextStep.bind(this));

Obselete.

https://codereview.chromium.org/1049483003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:140:
disable,
On 2015/04/02 01:34:54, Dan Beam wrote:
> can't this just be this.verifyDisableWorks in this case?  seems pretty
similar.
> 
> i didn't know you have to disable to test that enable works.

Yeah, can be - wasn't sure if that was too much overlap between the two tests
(since that means if disable fails, both will fail).  Changed.

[Dan Beam, 2015-04-06 18:51:55]

> and add test

> Also add a basic browsertest for some of the extensions page functionality.

next time (or now) can you split these into multiple CLs?

[Devlin, 2015-04-06 18:58:24]

On 2015/04/06 18:51:55, Dan Beam wrote:
> > and add test
> 
> > Also add a basic browsertest for some of the extensions page functionality.
> 
> next time (or now) can you split these into multiple CLs?

I suppose, sure - but usually submitting tests with the affected code is
preferred over doing it in separate patches (not least because it ensures the
submitted code works at all the first time).  Any reason to split them up?

[Dan Beam, 2015-04-06 22:56:52]

On 2015/04/06 18:58:24, Devlin wrote:
> On 2015/04/06 18:51:55, Dan Beam wrote:
> > > and add test
> > 
> > > Also add a basic browsertest for some of the extensions page
functionality.
> > 
> > next time (or now) can you split these into multiple CLs?
> 
> I suppose, sure - but usually submitting tests with the affected code is
> preferred over doing it in separate patches (not least because it ensures the
> submitted code works at all the first time).  

submit the tests first.

> Any reason to split them up?

so it's easier for me to review and so either your change or your test can be
reverted or merged separately.

[Devlin, 2015-04-06 23:50:43]

On 2015/04/06 22:56:52, Dan Beam wrote:
> On 2015/04/06 18:58:24, Devlin wrote:
> > On 2015/04/06 18:51:55, Dan Beam wrote:
> > > > and add test
> > > 
> > > > Also add a basic browsertest for some of the extensions page
> functionality.
> > > 
> > > next time (or now) can you split these into multiple CLs?
> > 
> > I suppose, sure - but usually submitting tests with the affected code is
> > preferred over doing it in separate patches (not least because it ensures
the
> > submitted code works at all the first time).  
> 
> submit the tests first.
> 
> > Any reason to split them up?
> 
> so it's easier for me to review and so either your change or your test can be
> reverted or merged separately.

Okay, done. :)

[Dan Beam, 2015-04-06 23:53:12]

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/extensio...
File chrome/browser/extensions/api/developer_private/developer_private_api.cc
(right):

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:413:
ExtensionInfoGenerator(browser_context()).CreateExtensionInfo(params->id);
+\s\s

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:415: if
(!info)
how is info ever empty?
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/extensio...
File chrome/browser/extensions/api/developer_private/extension_info_generator.cc
(right):

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/extension_info_generator.cc:371:
const Extension* extension = registry->enabled_extensions().GetByID(id);
nit: this is easier for me to read:

const Extension* enabled = registry->enabled_extensions().GetByID(id);
if (enabled)
  return CreateExtensionInfo(*enabled, developer::EXTENSION_STATE_ENABLED);

const Extension* disabled = registry->disabled_extensions().GetByID(id);
if (disabled)
  return CreateExtensionInfo(*disabled, developer::EXTENSION_STATE_DISABLED);

const Extension* terminated = registry->terminated_extensions().GetByID(id);
if (terminated)
  return CreateExtensionInfo(*terminated,
developer::EXTENSION_STATE_TERMINATED);

return scoped_ptr<api::developer_private::ExtensionInfo>();

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/resource...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:215:
this.extensionsUpdated_ = null;
1) why are you keeping a |this.extensionsUpdated_|?
2) why are you setting it to null here?

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:230: if
(eventData.extensionInfo)
when will |eventData.extensionInfo| not exist?

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:235: if (childNode)
when will childNode not exist?

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/extensions/extension_settings_browsertest.js
(right):

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:85:
this.steps.shift().bind(this)();
.call(this)

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:97: var
node = $(GOOD_CRX_ID);
$ -> getRequiredElement

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:108:
assertTrue(node != null);
getRequiredElement

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/extensions/extension_settings_handler.cc (right):

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_handler.cc:501:
Extension::DISABLE_NOT_VERIFIED) != 0)) {
nit: don't need the != 0

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_handler.cc:504: }
nit:

  if (ui_util::ShouldDisplayInExtensionSettings(extension.get(), profile)) {
    int disable_reasons = prefs->GetDisableReasons(extension->id());
    if (disable_reasons & Extension::DISABLE_NOT_VERIFIED) {
      should_do_verification_check = true;
      break;
    }
  }

[Dan Beam, 2015-04-06 23:55:48]

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/resource...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:215:
this.extensionsUpdated_ = null;
On 2015/04/06 23:53:12, Dan Beam wrote:
> 1) why are you keeping a |this.extensionsUpdated_|?
> 2) why are you setting it to null here?

ok, figured out 1) (using in tests), but why 2)?

[Devlin, 2015-04-07 16:01:46]

Patchset #5 (id:120001) has been deleted

[Devlin, 2015-04-07 16:02:17]

Patchset #5 (id:140001) has been deleted

[Devlin, 2015-04-07 16:07:54]

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/extensio...
File chrome/browser/extensions/api/developer_private/developer_private_api.cc
(right):

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:413:
ExtensionInfoGenerator(browser_context()).CreateExtensionInfo(params->id);
On 2015/04/06 23:53:11, Dan Beam wrote:
> +\s\s

Done.

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:415: if
(!info)
On 2015/04/06 23:53:12, Dan Beam wrote:
> how is info ever empty?
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...

Two methods. One takes a const Extension& (resultant info is never empty), one
takes an id (resultant info is empty iff an extension with |id| cannot be
found).

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/extensio...
File chrome/browser/extensions/api/developer_private/extension_info_generator.cc
(right):

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/extension_info_generator.cc:371:
const Extension* extension = registry->enabled_extensions().GetByID(id);
On 2015/04/06 23:53:12, Dan Beam wrote:
> nit: this is easier for me to read:
> 
> const Extension* enabled = registry->enabled_extensions().GetByID(id);
> if (enabled)
>   return CreateExtensionInfo(*enabled, developer::EXTENSION_STATE_ENABLED);
> 
> const Extension* disabled = registry->disabled_extensions().GetByID(id);
> if (disabled)
>   return CreateExtensionInfo(*disabled, developer::EXTENSION_STATE_DISABLED);
> 
> const Extension* terminated = registry->terminated_extensions().GetByID(id);
> if (terminated)
>   return CreateExtensionInfo(*terminated,
> developer::EXTENSION_STATE_TERMINATED);
> 
> return scoped_ptr<api::developer_private::ExtensionInfo>();

As you wish.

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/resource...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:215:
this.extensionsUpdated_ = null;
On 2015/04/06 23:55:48, Dan Beam wrote:
> On 2015/04/06 23:53:12, Dan Beam wrote:
> > 1) why are you keeping a |this.extensionsUpdated_|?
> > 2) why are you setting it to null here?
> 
> ok, figured out 1) (using in tests), but why 2)?

2 was because I thought it was best practice to initialize all variables
somewhere near object initialization (either in the proto or in a
constructor/initializer).  I judge by the fact you are asking that this is not
the case.  Removed.

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:230: if
(eventData.extensionInfo)
On 2015/04/06 23:53:12, Dan Beam wrote:
> when will |eventData.extensionInfo| not exist?

In the case of it being uninstalled, for one.  There are other times that if
something goes wonky, it could potentially happen.

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:235: if (childNode)
On 2015/04/06 23:53:12, Dan Beam wrote:
> when will childNode not exist?

In theory, never.  Removed.

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/extensions/extension_settings_browsertest.js
(right):

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:85:
this.steps.shift().bind(this)();
On 2015/04/06 23:53:12, Dan Beam wrote:
> .call(this)

Done.

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:97: var
node = $(GOOD_CRX_ID);
On 2015/04/06 23:53:12, Dan Beam wrote:
> $ -> getRequiredElement

Done.

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.js:108:
assertTrue(node != null);
On 2015/04/06 23:53:12, Dan Beam wrote:
> getRequiredElement

Done.

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/extensions/extension_settings_handler.cc (right):

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_handler.cc:501:
Extension::DISABLE_NOT_VERIFIED) != 0)) {
On 2015/04/06 23:53:12, Dan Beam wrote:
> nit: don't need the != 0

Last I checked, we do need the != 0 for windows machines.  Dumb, yes, but true.

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_handler.cc:504: }
On 2015/04/06 23:53:12, Dan Beam wrote:
> nit:
> 
>   if (ui_util::ShouldDisplayInExtensionSettings(extension.get(), profile)) {
>     int disable_reasons = prefs->GetDisableReasons(extension->id());
>     if (disable_reasons & Extension::DISABLE_NOT_VERIFIED) {
>       should_do_verification_check = true;
>       break;
>     }
>   }

Okay.

[Dan Beam, 2015-04-07 17:21:13]

lgtm

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/extensio...
File chrome/browser/extensions/api/developer_private/developer_private_api.cc
(right):

https://codereview.chromium.org/1049483003/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:415: if
(!info)
On 2015/04/07 16:07:54, Devlin wrote:
> On 2015/04/06 23:53:12, Dan Beam wrote:
> > how is info ever empty?
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...
> 
> Two methods. One takes a const Extension& (resultant info is never empty), one
> takes an id (resultant info is empty iff an extension with |id| cannot be
> found).

yeah, sorry, found the answer but didn't delete comment

https://codereview.chromium.org/1049483003/diff/160001/chrome/browser/resourc...
File chrome/browser/resources/extensions/extensions.js (right):

https://codereview.chromium.org/1049483003/diff/160001/chrome/browser/resourc...
chrome/browser/resources/extensions/extensions.js:343:
document.documentElement.classList.add('loading');
nit: out of curiosity, why not put this...

https://codereview.chromium.org/1049483003/diff/160001/chrome/browser/resourc...
chrome/browser/resources/extensions/extensions.js:473:
window.addEventListener('load', function(e) {
... here or something?

https://codereview.chromium.org/1049483003/diff/160001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/extensions/extension_settings_handler.cc (right):

https://codereview.chromium.org/1049483003/diff/160001/chrome/browser/ui/webu...
chrome/browser/ui/webui/extensions/extension_settings_handler.cc:501: if
((disable_reasons & Extension::DISABLE_NOT_VERIFIED) != 0) {
:(

[Devlin, 2015-04-07 21:14:51]

https://codereview.chromium.org/1049483003/diff/160001/chrome/browser/resourc...
File chrome/browser/resources/extensions/extensions.js (right):

https://codereview.chromium.org/1049483003/diff/160001/chrome/browser/resourc...
chrome/browser/resources/extensions/extensions.js:343:
document.documentElement.classList.add('loading');
On 2015/04/07 17:21:13, Dan Beam wrote:
> nit: out of curiosity, why not put this...

Sounds good to me.

https://codereview.chromium.org/1049483003/diff/160001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/extensions/extension_settings_handler.cc (right):

https://codereview.chromium.org/1049483003/diff/160001/chrome/browser/ui/webu...
chrome/browser/ui/webui/extensions/extension_settings_handler.cc:501: if
((disable_reasons & Extension::DISABLE_NOT_VERIFIED) != 0) {
On 2015/04/07 17:21:13, Dan Beam wrote:
> :(

Fixed.

[Devlin, 2015-04-07 21:14:56]

The CQ bit was checked by rdevlin.cronin@chromium.org

[Devlin, 2015-04-07 21:14:56]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/1049483003/#ps180001
(title: " ")

[commit-bot: I haz the power, 2015-04-07 21:15:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1049483003/180001

[commit-bot: I haz the power, 2015-04-07 22:52:30]

Committed patchset #6 (id:180001)

[commit-bot: I haz the power, 2015-04-07 22:53:20]

Patchset 6 (id:??) landed as
https://crrev.com/4ed20b10757af80bd743ede1742421c312503d14
Cr-Commit-Position: refs/heads/master@{#324140}