Assigning to tests author.

Eugene, did you repro these failures or did you get these from the bot? I didn't open this bug yet because I thought this failure may have been a system alert problem (and I just landed the code to handle that for the web shell), so I wanted to see how it went when those ran on flaky bot, which are currently purple.

Anyway, I'll look a little on Monday.

Did you mean to assign this to someone? I'll take it for now, but re-assign if you verified that there is a bug with these tests on device (and the owner isn't me).

I wanted to assign this to marq@ who wrote the tests.

I did not repro the failure, I just could not run tests on device and will try again on Monday. Log was taken from bots.

The URL is not available now so don't know about the previous failure.  I can constantly repro some failure(not sure if the same as bots) of ContextMenuTestCase on devices.  The failure is not caused by the system alert issue. 

When the "Copy Link" context menu shows up, tests will crash due to memory issue.

Test Case '-[ContextMenuTestCase testContextMenuWebkitTouchCalloutOverride]' started.
[INFO] GCDWebServer now reachable at http://Off-Road.local:8080/
Message from debugger: Terminated due to memory issue

Assign to marq@ who wrote the tests

Sylvain, Stepan, do you think OOM can be related to ARC changes?

ping!

I've tried running this test. Thanks to EG, I'm able to run with breakpoints. This line is where the memory spikes (from 150Mb to over 1Gb):

  // Context menu should have a "copy link" item.
  [[EarlGrey selectElementWithMatcher:copyItem]
      assertWithMatcher:grey_notNil()];

After examining it, I've found that the issue is a misplaced autorelease pool in EG internals. In GREYElementFinder:


Actual:

- (NSArray *)elementsMatchedInProvider:(id<GREYProvider>)elementProvider {
  NSParameterAssert(elementProvider);
  I_CHECK_MAIN_THREAD();

  NSMutableArray *matchingElements = [[NSMutableArray alloc] init];
  @autoreleasepool {
    for (id element in [elementProvider dataEnumerator]) {
      if ([_matcher matches:element]) {
        [matchingElements addObject:element];
      }
    }
  }
  return [NSArray arrayWithArray:matchingElements];
}

Intended:

- (NSArray *)elementsMatchedInProvider:(id<GREYProvider>)elementProvider {
  NSParameterAssert(elementProvider);
  I_CHECK_MAIN_THREAD();

  NSMutableArray *matchingElements = [[NSMutableArray alloc] init];
    for (id element in [elementProvider dataEnumerator]) {
      @autoreleasepool {
        if ([_matcher matches:element]) {
          [matchingElements addObject:element];
        }
      }
    }
  return [NSArray arrayWithArray:matchingElements];
}

With this change, the memory consumption does not peak when running this method.

I've attached before and after memory graphs. Also note how memory peaks twice before this test even runs. They should also be flaky on device, although I don't know what those tests are. 

I'll prepare a CL for EGTest.  

Deleted: before_eg_fix.png 32.1 KB

	before_eg_fix.png 32.1 KB View Download

Deleted: after_eg_fix.png 35.8 KB

	after_eg_fix.png 35.8 KB View Download

Sent https://github.com/google/EarlGrey/pull/391

The PR was merged, waiting for EG roll now, and it is presumably blocked on b/33761581, so assigning to Baxley. Please reassign to marq@ when the roll is unblocked or rolled. 

- CC self and sdefresne@ since the ARC question is resolved: presumably ARCing some code resulted in more autoreleased objects generated, and a misplaced autorelease pool failed to dispose of them promptly, hence memory issues. 

Thanks!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b7e5bd98c1437438f89230f581960e27c0e61b66

commit b7e5bd98c1437438f89230f581960e27c0e61b66
Author: baxley <baxley@chromium.org>
Date: Fri Jan 13 15:29:57 2017

Update EarlGrey bug references for device tests.

Page state and context menu tests were disabled and have more
specific bugs than the TODO contains.

BUG=675015, 675399 ,675247

Review-Url: https://codereview.chromium.org/2623333004
Cr-Commit-Position: refs/heads/master@{#443562}

[modify] https://crrev.com/b7e5bd98c1437438f89230f581960e27c0e61b66/ios/web/shell/test/context_menu_egtest.mm

I have a potential fix out to roll EarlGrey. I'll update this bug when there is an progress.

Thanks Mike!

This is blocked on 674583, which is preventing us from rolling EarlGrey.

That bug is waiting on a pull request being merged:
https://github.com/google/EarlGrey/pull/400

It is waiting on internal testing, which is in progress. Once that lands, we should be able to roll EarlGrey, which will fix these tests.

I'll update that bug as soon as I have any updates on the status of the pull request.

Thank you :)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/79f1ac6f7a24f1314f1e422bef9f63de460179fd

commit 79f1ac6f7a24f1314f1e422bef9f63de460179fd
Author: baxley <baxley@chromium.org>
Date: Sat Jan 28 00:36:14 2017

Re-enable web shell context menu EarlGrey tests.

The memory consumption problem was fixed in EarlGrey.

BUG= 675399 

Review-Url: https://codereview.chromium.org/2659163002
Cr-Commit-Position: refs/heads/master@{#446851}

[modify] https://crrev.com/79f1ac6f7a24f1314f1e422bef9f63de460179fd/ios/web/shell/test/context_menu_egtest.mm

We need to verify the stability of the EG roll before this gets cherry-picked to beta.

Regardless, the tests have been re-enabled on trunk, and from earlier investigation we knew it was a test issue, and not a product bug. So this may not need to block beta. Once we're okay with the EG roll, we can move this to beta.

[Auto-generated comment by a script] We noticed that this issue is targeted for M-57; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-57 label, otherwise remove Merge-TBD label. Thanks.