Implement the webNavigation.onCommitted event.

Implement the webNavigation.onCommitted event.

BUG=50943
TEST=ExtensionApiTest.WebNavigationEvents

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

[jochen (gone - plz use gerrit), 2010-09-06 17:46:32]

please review.

[Matt Perry, 2010-09-07 22:14:41]

LGTM after some small fixes.

http://codereview.chromium.org/3307013/diff/5001/6001
File chrome/browser/extensions/extension_webnavigation_api.cc (right):

http://codereview.chromium.org/3307013/diff/5001/6001#newcode77
chrome/browser/extensions/extension_webnavigation_api.cc:77: void
ExtensionWebNavigationEventRouter::DispatchEvent(
nit: in cases like this where the method doesn't depend on the class at all, I
prefer to make it a file-level static function.

http://codereview.chromium.org/3307013/diff/5001/6010
File chrome/common/page_transition_types.h (right):

http://codereview.chromium.org/3307013/diff/5001/6010#newcode123
chrome/common/page_transition_types.h:123: FORWARD_BACK = 0x01000000,
this belongs before CHAIN_START

http://codereview.chromium.org/3307013/diff/5001/6020
File chrome/test/data/extensions/api_test/webnavigation/navigation/test.html
(right):

http://codereview.chromium.org/3307013/diff/5001/6020#newcode1
chrome/test/data/extensions/api_test/webnavigation/navigation/test.html:1:
<script>
Nice tests!

http://codereview.chromium.org/3307013/diff/5001/6020#newcode20
chrome/test/data/extensions/api_test/webnavigation/navigation/test.html:20:
console.log('---onCommitted: ' + details.url);
2 space indent

http://codereview.chromium.org/3307013/diff/5001/6020#newcode32
chrome/test/data/extensions/api_test/webnavigation/navigation/test.html:32: var
tabId = tab.id;
2 space indent, same below.

http://codereview.chromium.org/3307013/diff/5001/6020#newcode90
chrome/test/data/extensions/api_test/webnavigation/navigation/test.html:90:
chrome.tabs.update(tabId, { url: getURL('forwardBack/b.html') });
I think you want to put this second call in the callback to the first update()
call, and only then if . Otherwise, it might run before the first is committed.
(In fact, I'm not 100% sure doing this will cause it to commit before running
the second call either. Watch this test for race flakiness).

[jochen (gone - plz use gerrit), 2010-09-08 08:44:50]

http://codereview.chromium.org/3307013/diff/5001/6001
File chrome/browser/extensions/extension_webnavigation_api.cc (right):

http://codereview.chromium.org/3307013/diff/5001/6001#newcode77
chrome/browser/extensions/extension_webnavigation_api.cc:77: void
ExtensionWebNavigationEventRouter::DispatchEvent(
On 2010/09/07 22:14:41, Matt Perry wrote:
> nit: in cases like this where the method doesn't depend on the class at all, I
> prefer to make it a file-level static function.
Given that the other event routers do the same, I'd rather stick with this.

http://codereview.chromium.org/3307013/diff/5001/6010
File chrome/common/page_transition_types.h (right):

http://codereview.chromium.org/3307013/diff/5001/6010#newcode123
chrome/common/page_transition_types.h:123: FORWARD_BACK = 0x01000000,
On 2010/09/07 22:14:41, Matt Perry wrote:
> this belongs before CHAIN_START

Done.

http://codereview.chromium.org/3307013/diff/5001/6020
File chrome/test/data/extensions/api_test/webnavigation/navigation/test.html
(right):

http://codereview.chromium.org/3307013/diff/5001/6020#newcode20
chrome/test/data/extensions/api_test/webnavigation/navigation/test.html:20:
console.log('---onCommitted: ' + details.url);
On 2010/09/07 22:14:41, Matt Perry wrote:
> 2 space indent

Done.

http://codereview.chromium.org/3307013/diff/5001/6020#newcode32
chrome/test/data/extensions/api_test/webnavigation/navigation/test.html:32: var
tabId = tab.id;
On 2010/09/07 22:14:41, Matt Perry wrote:
> 2 space indent, same below.

Done.

http://codereview.chromium.org/3307013/diff/5001/6020#newcode90
chrome/test/data/extensions/api_test/webnavigation/navigation/test.html:90:
chrome.tabs.update(tabId, { url: getURL('forwardBack/b.html') });
On 2010/09/07 22:14:41, Matt Perry wrote:
> I think you want to put this second call in the callback to the first update()
> call, and only then if . Otherwise, it might run before the first is
committed.
> (In fact, I'm not 100% sure doing this will cause it to commit before running
> the second call either. Watch this test for race flakiness).

Done.