Fixed a possible memory leak and support null pointers in AutomationEventObservers' NotifyEvent.

Fixed a possible memory leak and support null pointers in AutomationEventObservers' NotifyEvent.

BUG=None
TEST=functional/apptest.py


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=137466

[Nirnimesh, 2012-05-16 16:52:12]

Lgtm

On Wednesday, May 16, 2012,  <craigdh@chromium.org> wrote:
> http://codereview.chromium.org/10386144/
>

-- 
../NiR

[dennis_jeffrey, 2012-05-16 16:55:42]

A couple questions.

http://codereview.chromium.org/10386144/diff/1/chrome/browser/automation/auto...
File chrome/browser/automation/automation_event_observers.cc (right):

http://codereview.chromium.org/10386144/diff/1/chrome/browser/automation/auto...
chrome/browser/automation/automation_event_observers.cc:31: if (event_queue_) {
Will "value" get deleted on the "true" branch of this condition?

http://codereview.chromium.org/10386144/diff/1/chrome/browser/automation/auto...
chrome/browser/automation/automation_event_observers.cc:33: value = new
DictionaryValue;
Is this check needed?  It looks like both callers affected by this CL always
create a new DictionaryValue before invoking this function.  What about issuing
an error if the value isn't set by the caller?

[commit-bot: I haz the power, 2012-05-16 16:55:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/craigdh@chromium.org/10386144/1

[craigdh, 2012-05-16 17:00:55]

https://chromiumcodereview.appspot.com/10386144/diff/1/chrome/browser/automat...
File chrome/browser/automation/automation_event_observers.cc (right):

https://chromiumcodereview.appspot.com/10386144/diff/1/chrome/browser/automat...
chrome/browser/automation/automation_event_observers.cc:31: if (event_queue_) {
On 2012/05/16 16:55:42, dennis_jeffrey wrote:
> Will "value" get deleted on the "true" branch of this condition?

Yes. On "true" the AutomationEventQueue takes ownership.

https://chromiumcodereview.appspot.com/10386144/diff/1/chrome/browser/automat...
chrome/browser/automation/automation_event_observers.cc:33: value = new
DictionaryValue;
On 2012/05/16 16:55:42, dennis_jeffrey wrote:
> Is this check needed?  It looks like both callers affected by this CL always
> create a new DictionaryValue before invoking this function.  What about
issuing
> an error if the value isn't set by the caller?

Right now NULL is not being passed, but is because both current uses have
additional information to return. In general that seemed like an unnecessary
limitation and now the usage will be more similar to SendSuccess().

[dennis_jeffrey, 2012-05-16 17:03:08]

LGTM thanks!

[commit-bot: I haz the power, 2012-05-16 18:45:13]

Change committed as 137466