Remove special casing for firing load completes.

Remove special casing for firing load completes.

With the new logic in SendPendingAccessibilityNotifications, it is no longer necessary to fire a load complete  as the first notification to the browser. Since we walk the parent chain, looking for the first entry actually in our browser cache, we will always end up walking up to the root.

This change therefore also makes sure that notifications fired on the root will include all of the subtree when composing the notification.
BUG=none.
TEST=manual.

Merge remote-tracking branch 'origin' into build_break


Merge branch 'trunk' of http://git.chromium.org/git/chromium into build_break


Attempt fixes at chrome frame tests redness.

TBR=dtseng

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

[David Tseng, 2011-12-08 17:10:22]

dmazzoni: primary.
fyi: ananta.
owners: jam (move to "to" once reviewed).

[dmazzoni, 2011-12-08 17:54:30]

I'm testing this on Windows now...

http://codereview.chromium.org/8869006/diff/12001/content/renderer/renderer_a...
File content/renderer/renderer_accessibility.cc (right):

http://codereview.chromium.org/8869006/diff/12001/content/renderer/renderer_a...
content/renderer/renderer_accessibility.cc:222: root_id == notification.id;
Hmmm, I think this will send a lot of extraneous data; when I log accessibility
events I see a lot of events fired by WebKit on the root element and we don't
want to send a whole tree every time.

For now, how about sending a ChildrenChanged notification in the places where we
used to send LoadComplete?

But, if you see other problems with that, I'd be okay with:

1. Add a TODO
2. Submit this, hopefully fix chrome frame tests.
3. Follow this up by adding a clean mechanism to update the tree without sending
a notification.

[David Tseng, 2011-12-08 18:34:53]

>
http://codereview.chromium.org/8869006/diff/12001/content/renderer/renderer_a...
> File content/renderer/renderer_accessibility.cc (right):
>
>
http://codereview.chromium.org/8869006/diff/12001/content/renderer/renderer_a...
> content/renderer/renderer_accessibility.cc:222: root_id ==
> notification.id;
> Hmmm, I think this will send a lot of extraneous data; when I log
> accessibility events I see a lot of events fired by WebKit on the root
> element and we don't want to send a whole tree every time.
>
> For now, how about sending a ChildrenChanged notification in the places
> where we used to send LoadComplete?
>
> But, if you see other problems with that, I'd be okay with:
>
> 1. Add a TODO
> 2. Submit this, hopefully fix chrome frame tests.
> 3. Follow this up by adding a clean mechanism to update the tree without
> sending a notification.
>
> http://codereview.chromium.org/8869006/
>
Changing these layout completes to children changed and omitting the
root_id check to include children results in:
[4544:4876:2911821:FATAL:browser_accessibility_manager.cc(126)] Check
failed: renderer_id_to_child_id_map_[renderer_id] == child_id. graphic
775  Backtrace:
 base::debug::StackTrace::StackTrace [0x57D420F1+33]
(c:\chrome\src\base\debug\stack_trace_win.cc:149)
 logging::LogMessage::~LogMessage [0x57C8FCEF+63]
(c:\chrome\src\base\logging.cc:574)
 BrowserAccessibilityManager::Remove [0x5969D1B6+182]
(c:\chrome\src\content\browser\accessibility\browser_accessibility_manager.cc:129)

if you try tabbing into a page before it starts loading.

We can:
1.  add focused element changed to the list of should include
children. (not sure if this will work).
2.  keep this patch as is with a todo as you suggested.


It seems like we should include children whenever
PostAccessibilityNotifications walks up the parent chain (because the
current object is not in the browser/renderer map).

[dmazzoni, 2011-12-08 18:39:20]

On Thu, Dec 8, 2011 at 10:34 AM, David Tseng <dtseng@chromium.org> wrote:

> 2.  keep this patch as is with a todo as you suggested.
>

Let's do this.

Chrome Frame Tests are still running! Must be extra slow over remote
desktop. Some tests failed, but I think that more of them are succeeding
now, so this is a net win.

It seems like we should include children whenever
> PostAccessibilityNotifications walks up the parent chain (because the
> current object is not in the browser/renderer map).
>

Agreed. I think we're already doing this - see the line that explicitly
sets includes_children = true;

- Dominic

[dmazzoni, 2011-12-08 19:09:38]

Update: without this patch, all of the *CFTxtField* tests fail locally.
With this patch, all of these tests pass locally.

Probably a lot more tests are fixed, but those are the most definitive.

Check this in!