Split ExtensionTestObserver and move to //extensions.

Split ExtensionTestObserver and move to //extensions.

This CL splits ExtensionsTestObserver into two parts. The base class
exists in //extensions and has no dependencies on //chrome. The code that
needs to depend on //chrome is now in a subclass. This will allow us to
use the observer in tests that are out of //chrome.

This class is split but more code needs to be moved from
ExtensionChromeTestObserver to ExtensionTestObserver. There are certain
parts that we've left in the Chrome version simply because they are too
much work to move in this CL itself.

This CL is one of many that will be required to move tests for apps and
extensions API that are in //extensions to be tested without Chrome.

Unfortunately as a side effect of this CL, a lot of tests that did not
follow IWYU, started breaking on compile - since they were depending on
the include in extension_test_observer.h. I've added sky@ as a top level
reviewer for owners approval for the changes made to fix all of these
tests.

Reviews,
rdevlin.cronin@ - everything
sky@ - includes added to fix broken tests.

michaelpg@ - FYI

R=rdevlin.cronin@chromium.org, sky@chromium.org
BUG=505926
Committed: https://crrev.com/f65a72a71bea7aa1070c884e01ac24b91b45193f
Cr-Commit-Position: refs/heads/master@{#426304}

[Devlin, 2016-10-06 15:52:17]

Mostly just nits.

Can you also trim your CL description a bit?  Let's focus the commit notes on
what this CL addresses, and leave the bug link to describe the all the future
work.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/signin/oauth2_browsertest.cc (right):

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/signin/oauth2_browsertest.cc:768:
observer_.reset(new ExtensionBrowserTestNotificationObserver(browser));
iwyu?

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_browser_test_notification_observer.cc
(right):

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.cc:9:
#include "base/callback_list.h"
here too, please trim.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.cc:34:
typedef base::Callback<bool(void)> ConditionCallback;
prefer using syntax

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.cc:66: :
ExtensionTestNotificationObserver(GetBrowserContext()),
This is called before browser_ is set, so it will *always* initialize to
ProfileManager::GetActiveUserProfile().  That seems wrong?

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.cc:85:
extensions::ExtensionActionAPI::Get(GetBrowserContext())->AddObserver(this);
ScopedObserver?

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.cc:121:
base::Bind(&IsExtensionNotIdle, extension_id, GetBrowserContext()),
can we use a lambda here?

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.cc:127:
this->WaitForExtensionLoad();
no need for this->

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_browser_test_notification_observer.h
(right):

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.h:10:
#include "base/callback.h"
needed? (this and others; please trim)

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.h:25: 
Can this file be in the extensions:: namespace?

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.h:27:
class ExtensionBrowserTestNotificationObserver
nit: The common idiom is to name chrome versions ChromeFoo (see e.g.
ExtensionsClient and ChromeExtensionsClient, etc).  So this should be
ChromeExtensionTestNotificationObserver.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.h:31:
explicit ExtensionBrowserTestNotificationObserver(Browser*);
chrome style still requires variable naming in header files, afaik.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.h:34: //
Wait for the number of visible page actions to change to |count|.
Function descriptions should be descriptive, not imperative.  (line 37 is right,
but the rest need to be updated)

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.h:55:
Browser* browser_;
DISALLOW_COPY_AND_ASSIGN

https://codereview.chromium.org/2393343002/diff/1/chrome/test/BUILD.gn
File chrome/test/BUILD.gn (right):

https://codereview.chromium.org/2393343002/diff/1/chrome/test/BUILD.gn#newcod...
chrome/test/BUILD.gn:373:
"../browser/extensions/extension_browser_test_notification_observer.cc",
what about the header?

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
File extensions/test/extension_test_notification_observer.cc (right):

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.cc:10: #include
"content/public/browser/notification_registrar.h"
trim

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.cc:25: typedef
base::Callback<bool(void)> ConditionCallback;
using syntax

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.cc:132: return
crx_installers_done_observed_ == (before + 1);
no parens

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
File extensions/test/extension_test_notification_observer.h (right):

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.h:14: #include
"content/public/browser/notification_details.h"
Not all of these are needed, either.

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.h:26: // Test helper class
for observing extension-related events.
extensions:: namespace?

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.h:29: explicit
ExtensionTestNotificationObserver(content::BrowserContext*);
variable name

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.h:85:
base::CallbackList<void()>& callback_list() { return callback_list_; }
non-const & return values are discouraged by style.

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.h:102:
process_manager_observer_;
DISALLOW_COPY_AND_ASSIGN?

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.h:133: base::Closure
quit_closure_;
DISALLOW_COPY_AND_ASSIGN?

[sky, 2016-10-06 21:10:22]

LGTM

[Rahul Chaturvedi, 2016-10-11 20:12:35]

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/signin/oauth2_browsertest.cc (right):

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/signin/oauth2_browsertest.cc:768:
observer_.reset(new ExtensionBrowserTestNotificationObserver(browser));
On 2016/10/06 15:52:16, Devlin wrote:
> iwyu?

Done.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_browser_test_notification_observer.cc
(right):

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.cc:9:
#include "base/callback_list.h"
On 2016/10/06 15:52:16, Devlin wrote:
> here too, please trim.

Done.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.cc:34:
typedef base::Callback<bool(void)> ConditionCallback;
On 2016/10/06 15:52:16, Devlin wrote:
> prefer using syntax

Done.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.cc:66: :
ExtensionTestNotificationObserver(GetBrowserContext()),
On 2016/10/06 15:52:16, Devlin wrote:
> This is called before browser_ is set, so it will *always* initialize to
> ProfileManager::GetActiveUserProfile().  That seems wrong?

Whoops, initialization order.
Doesn't seem like a super elegant way to do this. Using a lambda here instead.
Its either that or a local anonymous namespace function. The lambda seems
cleaner.
Done.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.cc:85:
extensions::ExtensionActionAPI::Get(GetBrowserContext())->AddObserver(this);
On 2016/10/06 15:52:16, Devlin wrote:
> ScopedObserver?

Done.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.cc:121:
base::Bind(&IsExtensionNotIdle, extension_id, GetBrowserContext()),
On 2016/10/06 15:52:16, Devlin wrote:
> can we use a lambda here?

Done.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.cc:127:
this->WaitForExtensionLoad();
On 2016/10/06 15:52:16, Devlin wrote:
> no need for this->

Done.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_browser_test_notification_observer.h
(right):

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.h:10:
#include "base/callback.h"
On 2016/10/06 15:52:16, Devlin wrote:
> needed? (this and others; please trim)

Done.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.h:25: 
On 2016/10/06 15:52:17, Devlin wrote:
> Can this file be in the extensions:: namespace?

I am not sure why this wasn't in the first place, but this is just moved code. I
would rather not change something major like the namespace it belongs in.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.h:27:
class ExtensionBrowserTestNotificationObserver
On 2016/10/06 15:52:16, Devlin wrote:
> nit: The common idiom is to name chrome versions ChromeFoo (see e.g.
> ExtensionsClient and ChromeExtensionsClient, etc).  So this should be
> ChromeExtensionTestNotificationObserver.

Done.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.h:31:
explicit ExtensionBrowserTestNotificationObserver(Browser*);
On 2016/10/06 15:52:17, Devlin wrote:
> chrome style still requires variable naming in header files, afaik.

Done.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.h:34: //
Wait for the number of visible page actions to change to |count|.
On 2016/10/06 15:52:16, Devlin wrote:
> Function descriptions should be descriptive, not imperative.  (line 37 is
right,
> but the rest need to be updated)

Done.

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.h:55:
Browser* browser_;
On 2016/10/06 15:52:17, Devlin wrote:
> DISALLOW_COPY_AND_ASSIGN

Done.

https://codereview.chromium.org/2393343002/diff/1/chrome/test/BUILD.gn
File chrome/test/BUILD.gn (right):

https://codereview.chromium.org/2393343002/diff/1/chrome/test/BUILD.gn#newcod...
chrome/test/BUILD.gn:373:
"../browser/extensions/extension_browser_test_notification_observer.cc",
On 2016/10/06 15:52:17, Devlin wrote:
> what about the header?

Done.

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
File extensions/test/extension_test_notification_observer.cc (right):

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.cc:10: #include
"content/public/browser/notification_registrar.h"
On 2016/10/06 15:52:17, Devlin wrote:
> trim

Done.

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.cc:25: typedef
base::Callback<bool(void)> ConditionCallback;
On 2016/10/06 15:52:17, Devlin wrote:
> using syntax

Done.

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.cc:132: return
crx_installers_done_observed_ == (before + 1);
On 2016/10/06 15:52:17, Devlin wrote:
> no parens

Done.

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
File extensions/test/extension_test_notification_observer.h (right):

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.h:14: #include
"content/public/browser/notification_details.h"
On 2016/10/06 15:52:17, Devlin wrote:
> Not all of these are needed, either.

Done.

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.h:26: // Test helper class
for observing extension-related events.
On 2016/10/06 15:52:17, Devlin wrote:
> extensions:: namespace?

This is moved code, I would like to keep the original namespacing.

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.h:29: explicit
ExtensionTestNotificationObserver(content::BrowserContext*);
On 2016/10/06 15:52:17, Devlin wrote:
> variable name

Done.

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.h:85:
base::CallbackList<void()>& callback_list() { return callback_list_; }
On 2016/10/06 15:52:17, Devlin wrote:
> non-const & return values are discouraged by style.

Moved code.

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.h:102:
process_manager_observer_;
On 2016/10/06 15:52:17, Devlin wrote:
> DISALLOW_COPY_AND_ASSIGN?

Done.

https://codereview.chromium.org/2393343002/diff/1/extensions/test/extension_t...
extensions/test/extension_test_notification_observer.h:133: base::Closure
quit_closure_;
On 2016/10/06 15:52:17, Devlin wrote:
> DISALLOW_COPY_AND_ASSIGN?

Done.

[Devlin, 2016-10-11 22:53:46]

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_browser_test_notification_observer.h
(right):

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.h:25: 
On 2016/10/11 20:12:34, Rahul Chaturvedi wrote:
> On 2016/10/06 15:52:17, Devlin wrote:
> > Can this file be in the extensions:: namespace?
> 
> I am not sure why this wasn't in the first place, but this is just moved code.
I
> would rather not change something major like the namespace it belongs in.

Can you add a
// TODO(devlin): This should be in the extensions namespace.
?

https://codereview.chromium.org/2393343002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/signin/oauth2_browsertest.cc (right):

https://codereview.chromium.org/2393343002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/signin/oauth2_browsertest.cc:5: #include
<chrome/browser/extensions/chrome_extension_test_notification_observer.h>
This doesn't belong here.

https://codereview.chromium.org/2393343002/diff/40001/chrome/browser/extensio...
File chrome/browser/extensions/chrome_extension_test_notification_observer.cc
(right):

https://codereview.chromium.org/2393343002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/chrome_extension_test_notification_observer.cc:51:
[browser]() -> content::BrowserContext* {
I can't find anywhere we create one of these that doesn't pass in a browser. 
Are you sure there is one?  If not, let's just do browser_->profile().

https://codereview.chromium.org/2393343002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/chrome_extension_test_notification_observer.cc:63:
ChromeExtensionTestNotificationObserver::GetBrowserContext() {
no longer needed, right?  (Should always be context_)

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

https://codereview.chromium.org/2393343002/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.h:8: #include
<chrome/browser/extensions/chrome_extension_test_notification_observer.h>
again, wrong place

https://codereview.chromium.org/2393343002/diff/40001/extensions/test/extensi...
File extensions/test/extension_test_notification_observer.h (right):

https://codereview.chromium.org/2393343002/diff/40001/extensions/test/extensi...
extensions/test/extension_test_notification_observer.h:15: #include
"content/public/browser/notification_details.h"
forward declaration?

https://codereview.chromium.org/2393343002/diff/40001/extensions/test/extensi...
extensions/test/extension_test_notification_observer.h:18: #include
"content/public/browser/notification_types.h"
Where do we use this?

https://codereview.chromium.org/2393343002/diff/40001/extensions/test/extensi...
extensions/test/extension_test_notification_observer.h:19: #include
"extensions/browser/process_manager.h"
forward declaration?

https://codereview.chromium.org/2393343002/diff/40001/extensions/test/extensi...
extensions/test/extension_test_notification_observer.h:119:
content::BrowserContext* context_;
nit: prefer making this private and exposing content::BrowserContext* context()
{ return context_; }

[Rahul Chaturvedi, 2016-10-12 20:25:54]

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_browser_test_notification_observer.h
(right):

https://codereview.chromium.org/2393343002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_browser_test_notification_observer.h:25: 
On 2016/10/11 22:53:45, Devlin wrote:
> On 2016/10/11 20:12:34, Rahul Chaturvedi wrote:
> > On 2016/10/06 15:52:17, Devlin wrote:
> > > Can this file be in the extensions:: namespace?
> > 
> > I am not sure why this wasn't in the first place, but this is just moved
code.
> I
> > would rather not change something major like the namespace it belongs in.
> 
> Can you add a
> // TODO(devlin): This should be in the extensions namespace.
> ?

Done.

https://codereview.chromium.org/2393343002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/signin/oauth2_browsertest.cc (right):

https://codereview.chromium.org/2393343002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/signin/oauth2_browsertest.cc:5: #include
<chrome/browser/extensions/chrome_extension_test_notification_observer.h>
On 2016/10/11 22:53:45, Devlin wrote:
> This doesn't belong here.

Eclipse screwup.
Fixed.

https://codereview.chromium.org/2393343002/diff/40001/chrome/browser/extensio...
File chrome/browser/extensions/chrome_extension_test_notification_observer.cc
(right):

https://codereview.chromium.org/2393343002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/chrome_extension_test_notification_observer.cc:51:
[browser]() -> content::BrowserContext* {
On 2016/10/11 22:53:45, Devlin wrote:
> I can't find anywhere we create one of these that doesn't pass in a browser. 
> Are you sure there is one?  If not, let's just do browser_->profile().

Done.

https://codereview.chromium.org/2393343002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/chrome_extension_test_notification_observer.cc:63:
ChromeExtensionTestNotificationObserver::GetBrowserContext() {
On 2016/10/11 22:53:45, Devlin wrote:
> no longer needed, right?  (Should always be context_)

Done.

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

https://codereview.chromium.org/2393343002/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_settings_browsertest.h:8: #include
<chrome/browser/extensions/chrome_extension_test_notification_observer.h>
On 2016/10/11 22:53:45, Devlin wrote:
> again, wrong place

Done.

https://codereview.chromium.org/2393343002/diff/40001/extensions/test/extensi...
File extensions/test/extension_test_notification_observer.h (right):

https://codereview.chromium.org/2393343002/diff/40001/extensions/test/extensi...
extensions/test/extension_test_notification_observer.h:15: #include
"content/public/browser/notification_details.h"
On 2016/10/11 22:53:45, Devlin wrote:
> forward declaration?

Done.

https://codereview.chromium.org/2393343002/diff/40001/extensions/test/extensi...
extensions/test/extension_test_notification_observer.h:18: #include
"content/public/browser/notification_types.h"
On 2016/10/11 22:53:45, Devlin wrote:
> Where do we use this?

We aren't. Looks like a lot of other tests are though.
Removed and fixed a few more tests for IWYU.

https://codereview.chromium.org/2393343002/diff/40001/extensions/test/extensi...
extensions/test/extension_test_notification_observer.h:19: #include
"extensions/browser/process_manager.h"
On 2016/10/11 22:53:46, Devlin wrote:
> forward declaration?

Done.

https://codereview.chromium.org/2393343002/diff/40001/extensions/test/extensi...
extensions/test/extension_test_notification_observer.h:119:
content::BrowserContext* context_;
On 2016/10/11 22:53:46, Devlin wrote:
> nit: prefer making this private and exposing content::BrowserContext*
context()
> { return context_; }

Done.

[Devlin, 2016-10-12 22:51:19]

lgtm as long as the bots are happy

[Rahul Chaturvedi, 2016-10-12 22:53:31]

The CQ bit was checked by steel@chromium.org

[Rahul Chaturvedi, 2016-10-12 22:53:32]

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

[commit-bot: I haz the power, 2016-10-12 22:54:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-12 23:08:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-12 23:08:40]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Rahul Chaturvedi, 2016-10-18 19:40:34]

The CQ bit was checked by steel@chromium.org

[Rahul Chaturvedi, 2016-10-18 19:40:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org, sky@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/2393343002/#ps80001
(title: "moar iwyu test fixes")

[commit-bot: I haz the power, 2016-10-18 19:40:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-18 20:22:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-18 20:22:43]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Rahul Chaturvedi, 2016-10-18 21:00:26]

The CQ bit was checked by steel@chromium.org

[Rahul Chaturvedi, 2016-10-18 21:00:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2393343002/#ps120001
(title: ".")

[commit-bot: I haz the power, 2016-10-18 21:00:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-18 21:57:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-18 21:57:09]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Rahul Chaturvedi, 2016-10-18 23:31:10]

The CQ bit was checked by steel@chromium.org

[commit-bot: I haz the power, 2016-10-18 23:32:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-19 06:05:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-19 06:05:38]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Rahul Chaturvedi, 2016-10-19 20:46:08]

The CQ bit was checked by steel@chromium.org

[Rahul Chaturvedi, 2016-10-19 20:46:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2393343002/#ps140001
(title: "fix bug in ActivityLogApiTest.TriggerEvent")

[commit-bot: I haz the power, 2016-10-19 20:46:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-19 22:17:13]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-10-21 13:12:08]

Description was changed from

==========
Split ExtensionTestObserver and move to //extensions.

This CL splits ExtensionsTestObserver into two parts. The base class
exists in //extensions and has no dependencies on //chrome. The code that
needs to depend on //chrome is now in a subclass. This will allow us to
use the observer in tests that are out of //chrome.

This class is split but more code needs to be moved from
ExtensionChromeTestObserver to ExtensionTestObserver. There are certain
parts that we've left in the Chrome version simply because they are too
much work to move in this CL itself.

This CL is one of many that will be required to move tests for apps and
extensions API that are in //extensions to be tested without Chrome.

Unfortunately as a side effect of this CL, a lot of tests that did not
follow IWYU, started breaking on compile - since they were depending on
the include in extension_test_observer.h. I've added sky@ as a top level
reviewer for owners approval for the changes made to fix all of these
tests.

Reviews,
rdevlin.cronin@ - everything
sky@ - includes added to fix broken tests.

michaelpg@ - FYI

R=rdevlin.cronin@chromium.org, sky@chromium.org
BUG=505926
==========

to

==========
Split ExtensionTestObserver and move to //extensions.

This CL splits ExtensionsTestObserver into two parts. The base class
exists in //extensions and has no dependencies on //chrome. The code that
needs to depend on //chrome is now in a subclass. This will allow us to
use the observer in tests that are out of //chrome.

This class is split but more code needs to be moved from
ExtensionChromeTestObserver to ExtensionTestObserver. There are certain
parts that we've left in the Chrome version simply because they are too
much work to move in this CL itself.

This CL is one of many that will be required to move tests for apps and
extensions API that are in //extensions to be tested without Chrome.

Unfortunately as a side effect of this CL, a lot of tests that did not
follow IWYU, started breaking on compile - since they were depending on
the include in extension_test_observer.h. I've added sky@ as a top level
reviewer for owners approval for the changes made to fix all of these
tests.

Reviews,
rdevlin.cronin@ - everything
sky@ - includes added to fix broken tests.

michaelpg@ - FYI

R=rdevlin.cronin@chromium.org, sky@chromium.org
BUG=505926

Committed: https://crrev.com/f65a72a71bea7aa1070c884e01ac24b91b45193f
Cr-Commit-Position: refs/heads/master@{#426304}
==========

[commit-bot: I haz the power, 2016-10-21 13:12:12]

Patchset 8 (id:??) landed as
https://crrev.com/f65a72a71bea7aa1070c884e01ac24b91b45193f
Cr-Commit-Position: refs/heads/master@{#426304}