Forward accessibility events to the automation extension process.

Forward accessibility events to the automation extension process.

See bug for context. This patch adds code to
keep track of extensions using the automation API
that want to subscribe to accessibility events and
implements an IPC to forward the accessibility events.

The next CL will complete the implementation
based on this new IPC and delete the code that
forwards accessibility events using the extension
event router.

BUG=495323
Committed: https://crrev.com/a6ea68fcf2fb2de16fbe59fa731fb2b75b96c14d
Cr-Commit-Position: refs/heads/master@{#333616}

[dmazzoni, 2015-06-02 21:54:32]

dmazzoni@chromium.org changed reviewers:
+ dcheng@chromium.org, jam@chromium.org, kalman@chromium.org

[dmazzoni, 2015-06-02 21:54:32]

kalman, dtseng, aboxhall:
  Primary / all extensions and automation API code
jam:
  components/
dcheng:
  chrome/common/extensions/chrome_extension_messages.h
  content/common/accessibility_messages.h
  content/public/common/common_param_traits_macros.h

[not at google - send to devlin, 2015-06-02 23:59:25]

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
File
chrome/browser/extensions/api/automation_internal/automation_event_router.cc
(right):

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:57:
AutomationListener* listener;
it's nice to initialize pointers to nullptr

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:59:
listener = iter->second;
What happens if the listener that is already there has a different routing ID?
or ax_tree_id? or whatever? I find the contract of this method, whatever it is,
confusing.

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:65:
listener_map_.insert(std::make_pair(extension_id, listener));
I would have thought that listener_map_[extension_id] = listener is easier to
write?

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:74:
++iter) {
You can use a range-based for loop here, I think?

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:99:
if (browser_context && EventRouter::Get(browser_context)) {
Checking for |browser_context| here implies it can be null, could you document
why?

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:123:
auto temp = iter;
apologies for nit, but for what it's worth I've more commonly seen this written
as:

for (auto iter = listener_map_.begin(); iter != listener_map_.end();) {
  if (iter->second->process_id = process_id)
    listener_map_.erase(iter++);
  else
    ++iter;
}

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/automation_internal/automation_event_router.h
(right):

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_event_router.h:32:
void AddListener(ExtensionId extension_id,
mention when it's removed.

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
File
chrome/browser/extensions/api/automation_internal/automation_internal_api.cc
(right):

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_internal_api.cc:365:
router->AddListener(extension()->id(),
you might want to comment when this gets removed, and that said, the
AutomationEventRouter does need a number of comments IMO.

https://codereview.chromium.org/1151523009/diff/1/chrome/common/extensions/ap...
File chrome/common/extensions/api/automation_internal.idl (right):

https://codereview.chromium.org/1151523009/diff/1/chrome/common/extensions/ap...
chrome/common/extensions/api/automation_internal.idl:106: optional long tabId,
This is going to get a bit confusing with the 2 ints, can you change it to be a
dictionary EnableTabParams?

Also I don't actually see the routing ID used in the implementation.

https://codereview.chromium.org/1151523009/diff/1/chrome/renderer/extensions/...
File chrome/renderer/extensions/automation_internal_custom_bindings.cc (right):

https://codereview.chromium.org/1151523009/diff/1/chrome/renderer/extensions/...
chrome/renderer/extensions/automation_internal_custom_bindings.cc:61:
IPC_MESSAGE_UNHANDLED(CHECK(false) << "Unhandled IPC message")
How does this work out?

(btw, personally I prefer LOG(FATAL) because CHECK(false) always seemed like an
oxymoron, but, that's just me :)

https://codereview.chromium.org/1151523009/diff/1/chrome/renderer/extensions/...
File chrome/renderer/extensions/automation_internal_custom_bindings.h (right):

https://codereview.chromium.org/1151523009/diff/1/chrome/renderer/extensions/...
chrome/renderer/extensions/automation_internal_custom_bindings.h:8: #include
<vector>
unused? also the scoped_ptr include and maybe the hash_tables include?

https://codereview.chromium.org/1151523009/diff/1/chrome/renderer/extensions/...
chrome/renderer/extensions/automation_internal_custom_bindings.h:25: public
ChromeV8ExtensionHandler {
ChromeV8ExtensionHandler is not a nice class, it's unnecessary as a superclass
(and in this case breaks multiple inheritance rules).

(the history is that most of its functionality has either been deleted or moved
into ObjectBackedNativeHandler, except for that routing ID stuff. It has nothing
to do with v8 anymore)

Proposal: pull its functionality into a composable object, let's say
IPCEndpoint:

class IPCEndpoint {
 public:
  explicit IPCEndpoint(IPC::Listener* listener);
  int GetRoutingID();
  void Send(IPC::Message* message);
 private:
  int routing_id_;
}

IPCEndpoint::IPCEndpoint() : routing_id_(-1) {}
// More or less the contents of chrome_v8_extension_handler.

Then have an IPCEndpoint as a member variable of this class to send to, and make
this class extend IPC::Listener instead of ChromeV8ExtensionHandler.

I wonder if that would let me delete ChromeV8ExtensionHandler actually.

[jam, 2015-06-03 04:05:55]

lgtm

[dmazzoni, 2015-06-04 20:07:39]

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
File
chrome/browser/extensions/api/automation_internal/automation_event_router.cc
(right):

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:57:
AutomationListener* listener;
On 2015/06/02 23:59:24, kalman wrote:
> it's nice to initialize pointers to nullptr

Done.

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:59:
listener = iter->second;
On 2015/06/02 23:59:24, kalman wrote:
> What happens if the listener that is already there has a different routing ID?
> or ax_tree_id? or whatever? I find the contract of this method, whatever it
is,
> confusing.

OK, I tried to clarify it. I realized it was a mistake to key on the extension
ID because there can be two extension processes corresponding to the same ID in
different browser contexts. I changed it to a vector with the process id and
routing id uniquely identifying the listener, and the other parameters
identifying the trees it wants to listen to.

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:65:
listener_map_.insert(std::make_pair(extension_id, listener));
On 2015/06/02 23:59:24, kalman wrote:
> I would have thought that listener_map_[extension_id] = listener is easier to
> write?

Doesn't compile, but moot as I simplified it.

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:74:
++iter) {
On 2015/06/02 23:59:24, kalman wrote:
> You can use a range-based for loop here, I think?

Done.

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:99:
if (browser_context && EventRouter::Get(browser_context)) {
On 2015/06/02 23:59:24, kalman wrote:
> Checking for |browser_context| here implies it can be null, could you document
> why?

Hmmm, I can't see a good reason why. The caller should just handle
AXTreeIDRegistry::GetInstance()->RemoveAXTreeID instead of this function if
there's no browser context.

Check it now.

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:123:
auto temp = iter;
On 2015/06/02 23:59:24, kalman wrote:
> apologies for nit, but for what it's worth I've more commonly seen this
written
> as:
> 
> for (auto iter = listener_map_.begin(); iter != listener_map_.end();) {
>   if (iter->second->process_id = process_id)
>     listener_map_.erase(iter++);
>   else
>     ++iter;
> }

Thanks, that's cleaner.

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/automation_internal/automation_event_router.h
(right):

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_event_router.h:32:
void AddListener(ExtensionId extension_id,
On 2015/06/02 23:59:24, kalman wrote:
> mention when it's removed.

Done. Note that the current automation API doesn't seem to have a way to stop
listening. Maybe we should add it, but for now I'm just trying to reimplement
what's already there.

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
File
chrome/browser/extensions/api/automation_internal/automation_internal_api.cc
(right):

https://codereview.chromium.org/1151523009/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/automation_internal/automation_internal_api.cc:365:
router->AddListener(extension()->id(),
On 2015/06/02 23:59:24, kalman wrote:
> you might want to comment when this gets removed, and that said, the
> AutomationEventRouter does need a number of comments IMO.

Done.

https://codereview.chromium.org/1151523009/diff/1/chrome/common/extensions/ap...
File chrome/common/extensions/api/automation_internal.idl (right):

https://codereview.chromium.org/1151523009/diff/1/chrome/common/extensions/ap...
chrome/common/extensions/api/automation_internal.idl:106: optional long tabId,
On 2015/06/02 23:59:24, kalman wrote:
> This is going to get a bit confusing with the 2 ints, can you change it to be
a
> dictionary EnableTabParams?

Sure, done.

> Also I don't actually see the routing ID used in the implementation.

Oops, forgot to include that bit in this change. Done.

https://codereview.chromium.org/1151523009/diff/1/chrome/renderer/extensions/...
File chrome/renderer/extensions/automation_internal_custom_bindings.cc (right):

https://codereview.chromium.org/1151523009/diff/1/chrome/renderer/extensions/...
chrome/renderer/extensions/automation_internal_custom_bindings.cc:61:
IPC_MESSAGE_UNHANDLED(CHECK(false) << "Unhandled IPC message")
On 2015/06/02 23:59:25, kalman wrote:
> How does this work out?
> 
> (btw, personally I prefer LOG(FATAL) because CHECK(false) always seemed like
an
> oxymoron, but, that's just me :)

Moot now that this is a MessageFilter.

https://codereview.chromium.org/1151523009/diff/1/chrome/renderer/extensions/...
File chrome/renderer/extensions/automation_internal_custom_bindings.h (right):

https://codereview.chromium.org/1151523009/diff/1/chrome/renderer/extensions/...
chrome/renderer/extensions/automation_internal_custom_bindings.h:8: #include
<vector>
On 2015/06/02 23:59:25, kalman wrote:
> unused? also the scoped_ptr include and maybe the hash_tables include?

Done.

https://codereview.chromium.org/1151523009/diff/1/chrome/renderer/extensions/...
chrome/renderer/extensions/automation_internal_custom_bindings.h:25: public
ChromeV8ExtensionHandler {
On 2015/06/02 23:59:25, kalman wrote:
> ChromeV8ExtensionHandler is not a nice class, it's unnecessary as a superclass
> (and in this case breaks multiple inheritance rules).
> 
> (the history is that most of its functionality has either been deleted or
moved
> into ObjectBackedNativeHandler, except for that routing ID stuff. It has
nothing
> to do with v8 anymore)
> 
> Proposal: pull its functionality into a composable object, let's say
> IPCEndpoint:
> 
> class IPCEndpoint {
>  public:
>   explicit IPCEndpoint(IPC::Listener* listener);
>   int GetRoutingID();
>   void Send(IPC::Message* message);
>  private:
>   int routing_id_;
> }
> 
> IPCEndpoint::IPCEndpoint() : routing_id_(-1) {}
> // More or less the contents of chrome_v8_extension_handler.
> 
> Then have an IPCEndpoint as a member variable of this class to send to, and
make
> this class extend IPC::Listener instead of ChromeV8ExtensionHandler.
> 
> I wonder if that would let me delete ChromeV8ExtensionHandler actually.

As discussed I'll go with a MessageFilter instead, there's no reason we need a
separate routing id here.

[not at google - send to devlin, 2015-06-04 23:03:29]

lgtm with some minor factoring comments.

https://codereview.chromium.org/1151523009/diff/60001/chrome/browser/extensio...
File chrome/browser/extensions/api/automation_internal/automation_event_router.h
(right):

https://codereview.chromium.org/1151523009/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/automation_internal/automation_event_router.h:36:
void AddListener(int listener_process_id,
A name like "AddListener" implies to me that it should take a Listener or a
base::Callback as a parameter, i.e. that the listener is an actual object. Maybe
"Register" or something at least. Or "StartForwarding".

https://codereview.chromium.org/1151523009/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/automation_internal/automation_event_router.h:39:
bool desktop);
On reading call sites, I'd find it nicer if this were an enum of DESKTOP and
TAB. Or alternatively 2 different methods
StartForwardingFromTab/StartForwardingFromDesktop (or whatever) since only the
Tab version needs the tree ID right?

https://codereview.chromium.org/1151523009/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/automation_internal/automation_event_router.h:41:
void DispatchAccessibilityEventToAutomation(
Well I'd drop the "ToAutomation", but up to you.

https://codereview.chromium.org/1151523009/diff/60001/chrome/common/extension...
File chrome/common/extensions/chrome_extension_messages.h (right):

https://codereview.chromium.org/1151523009/diff/60001/chrome/common/extension...
chrome/common/extensions/chrome_extension_messages.h:73: 
extra \n

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/automation_custom_bindings.js (right):

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/automation_custom_bindings.js:17: var
GetRoutingID = requireNative('automationInternal').GetRoutingID;
rather than requiring automationInternal 3 times, require it once then use
automationInternal.GetRoutingID(), automationInternal.GetSchemaAdditions(), etc.

[dcheng, 2015-06-06 00:00:52]

https://codereview.chromium.org/1151523009/diff/60001/chrome/browser/extensio...
File
chrome/browser/extensions/api/automation_internal/automation_event_router.cc
(right):

https://codereview.chromium.org/1151523009/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:50:
[=](AutomationListener& item) {
We don't currently permit default captures: http://chromium-cpp.appspot.com/
(search for default captures)

https://codereview.chromium.org/1151523009/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:75:
for (auto iter : listeners_) {
Doesn't this make a copy of AutomationListener on each iteration? Consider using
const auto&

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/extensi...
File chrome/renderer/extensions/automation_internal_custom_bindings.cc (right):

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/extensi...
chrome/renderer/extensions/automation_internal_custom_bindings.cc:45: :
owner_(owner) {
Is it valid to call this constructor with no owner? It doesn't look like we ever
do, so maybe DCHECK(owner)?

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/extensi...
chrome/renderer/extensions/automation_internal_custom_bindings.cc:49: void
Detach() {
Who calls Detach()?

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/extensi...
chrome/renderer/extensions/automation_internal_custom_bindings.cc:63:
content::RenderThread* render_thread = content::RenderThread::Get();
Why would this be null? To me, this conditional makes it seem like we're not
sure when this object dies in relation to the RenderThread?

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/extensi...
chrome/renderer/extensions/automation_internal_custom_bindings.cc:68:
AutomationInternalCustomBindings* owner_;
Since we don't use Detach() anymore, make this an
AutomationInternalCustomBindings* const owner_?

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/extensi...
chrome/renderer/extensions/automation_internal_custom_bindings.cc:86:
base::Unretained(this)));
In general, it's nice to document why base::Unretained() is safe.

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/extensi...
chrome/renderer/extensions/automation_internal_custom_bindings.cc:146: //
TODO(dmazzoni): finish implementing this.
Please add me as a reviewer on the change where you finish implementing this.

https://codereview.chromium.org/1151523009/diff/60001/extensions/browser/exte...
File extensions/browser/extension_function.h (right):

https://codereview.chromium.org/1151523009/diff/60001/extensions/browser/exte...
extensions/browser/extension_function.h:284: int source_process_id() {
Nit: const

[dmazzoni, 2015-06-08 18:02:58]

https://codereview.chromium.org/1151523009/diff/60001/chrome/browser/extensio...
File
chrome/browser/extensions/api/automation_internal/automation_event_router.cc
(right):

https://codereview.chromium.org/1151523009/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:50:
[=](AutomationListener& item) {
On 2015/06/06 00:00:52, dcheng wrote:
> We don't currently permit default captures: http://chromium-cpp.appspot.com/
> (search for default captures)

Thanks. Still getting used to C++11 style rules.

https://codereview.chromium.org/1151523009/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:75:
for (auto iter : listeners_) {
On 2015/06/06 00:00:52, dcheng wrote:
> Doesn't this make a copy of AutomationListener on each iteration? Consider
using
> const auto&

Done.

https://codereview.chromium.org/1151523009/diff/60001/chrome/browser/extensio...
File chrome/browser/extensions/api/automation_internal/automation_event_router.h
(right):

https://codereview.chromium.org/1151523009/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/automation_internal/automation_event_router.h:36:
void AddListener(int listener_process_id,
On 2015/06/04 23:03:28, kalman wrote:
> A name like "AddListener" implies to me that it should take a Listener or a
> base::Callback as a parameter, i.e. that the listener is an actual object.
Maybe
> "Register" or something at least. Or "StartForwarding".

I like "Register", since the name of the class (AutomationEventListener)
explains a lot of what it means now.

https://codereview.chromium.org/1151523009/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/automation_internal/automation_event_router.h:39:
bool desktop);
On 2015/06/04 23:03:28, kalman wrote:
> On reading call sites, I'd find it nicer if this were an enum of DESKTOP and
> TAB. Or alternatively 2 different methods
> StartForwardingFromTab/StartForwardingFromDesktop (or whatever) since only the
> Tab version needs the tree ID right?

Sure. Made into two methods.

https://codereview.chromium.org/1151523009/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/automation_internal/automation_event_router.h:41:
void DispatchAccessibilityEventToAutomation(
On 2015/06/04 23:03:28, kalman wrote:
> Well I'd drop the "ToAutomation", but up to you.

Done.

https://codereview.chromium.org/1151523009/diff/60001/chrome/common/extension...
File chrome/common/extensions/chrome_extension_messages.h (right):

https://codereview.chromium.org/1151523009/diff/60001/chrome/common/extension...
chrome/common/extensions/chrome_extension_messages.h:73: 
On 2015/06/04 23:03:29, kalman wrote:
> extra \n

Done.

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/extensi...
File chrome/renderer/extensions/automation_internal_custom_bindings.cc (right):

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/extensi...
chrome/renderer/extensions/automation_internal_custom_bindings.cc:45: :
owner_(owner) {
On 2015/06/06 00:00:52, dcheng wrote:
> Is it valid to call this constructor with no owner? It doesn't look like we
ever
> do, so maybe DCHECK(owner)?

Done.

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/extensi...
chrome/renderer/extensions/automation_internal_custom_bindings.cc:49: void
Detach() {
On 2015/06/06 00:00:52, dcheng wrote:
> Who calls Detach()?

Oops, meant to call this in ~AutomationInternalCustomBindings(). Good catch.

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/extensi...
chrome/renderer/extensions/automation_internal_custom_bindings.cc:63:
content::RenderThread* render_thread = content::RenderThread::Get();
On 2015/06/06 00:00:52, dcheng wrote:
> Why would this be null? To me, this conditional makes it seem like we're not
> sure when this object dies in relation to the RenderThread?

Looks like it shouldn't be null, removed the check.

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/extensi...
chrome/renderer/extensions/automation_internal_custom_bindings.cc:68:
AutomationInternalCustomBindings* owner_;
On 2015/06/06 00:00:52, dcheng wrote:
> Since we don't use Detach() anymore, make this an
> AutomationInternalCustomBindings* const owner_?

I think we need Detach() because the message filter is refcounted and can
outlive the owner.

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/extensi...
chrome/renderer/extensions/automation_internal_custom_bindings.cc:86:
base::Unretained(this)));
On 2015/06/06 00:00:52, dcheng wrote:
> In general, it's nice to document why base::Unretained() is safe.

Done.

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/extensi...
chrome/renderer/extensions/automation_internal_custom_bindings.cc:146: //
TODO(dmazzoni): finish implementing this.
On 2015/06/06 00:00:52, dcheng wrote:
> Please add me as a reviewer on the change where you finish implementing this.

Done: https://codereview.chromium.org/1155183006/

It's basically done and working now, I'm just trying to clean it up and get it
reviewed in pieces.

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/automation_custom_bindings.js (right):

https://codereview.chromium.org/1151523009/diff/60001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/automation_custom_bindings.js:17: var
GetRoutingID = requireNative('automationInternal').GetRoutingID;
On 2015/06/04 23:03:29, kalman wrote:
> rather than requiring automationInternal 3 times, require it once then use
> automationInternal.GetRoutingID(), automationInternal.GetSchemaAdditions(),
etc.

Done.

https://codereview.chromium.org/1151523009/diff/60001/extensions/browser/exte...
File extensions/browser/extension_function.h (right):

https://codereview.chromium.org/1151523009/diff/60001/extensions/browser/exte...
extensions/browser/extension_function.h:284: int source_process_id() {
On 2015/06/06 00:00:52, dcheng wrote:
> Nit: const

Done.

[dcheng, 2015-06-08 18:07:05]

lgtm with a nit

https://codereview.chromium.org/1151523009/diff/100001/chrome/browser/extensi...
File
chrome/browser/extensions/api/automation_internal/automation_event_router.cc
(right):

https://codereview.chromium.org/1151523009/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:134:
[=](AutomationListener& item) {
This lambda is using default capture too

[dmazzoni, 2015-06-08 18:45:18]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2015-06-08 18:45:18]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org,
kalman@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/1151523009/#ps120001
(title: "Fix one more lambda capture")

[dmazzoni, 2015-06-08 18:45:57]

https://codereview.chromium.org/1151523009/diff/100001/chrome/browser/extensi...
File
chrome/browser/extensions/api/automation_internal/automation_event_router.cc
(right):

https://codereview.chromium.org/1151523009/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/automation_internal/automation_event_router.cc:134:
[=](AutomationListener& item) {
On 2015/06/08 18:07:05, dcheng wrote:
> This lambda is using default capture too

Done.

[commit-bot: I haz the power, 2015-06-08 18:48:13]

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

[dmazzoni, 2015-06-08 19:06:36]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2015-06-08 19:06:36]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org,
dcheng@chromium.org, kalman@chromium.org
Link to the patchset: https://codereview.chromium.org/1151523009/#ps140001
(title: "Fix typo in GetSchemaAdditions")

[commit-bot: I haz the power, 2015-06-08 19:08:27]

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

[commit-bot: I haz the power, 2015-06-08 20:08:13]

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

[commit-bot: I haz the power, 2015-06-08 20:08:16]

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

[dmazzoni, 2015-06-09 22:52:07]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2015-06-09 22:52:08]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org,
dcheng@chromium.org, kalman@chromium.org
Link to the patchset: https://codereview.chromium.org/1151523009/#ps160001
(title: "Handle case where RenderThread destroys AutomationMessageFilter first")

[commit-bot: I haz the power, 2015-06-09 22:52:31]

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

[commit-bot: I haz the power, 2015-06-10 00:17:17]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-06-10 00:18:15]

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